| Message ID | 20171201151538.6844-2-farman@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 01, 2017 at 04:15:38PM +0100, Eric Farman wrote: > Commit 5c0919d02066 ("virtio-scsi: Add virtqueue_size parameter allowing > virtqueue size to be set.") introduced a new parameter to virtio-scsi. > Later, commit 920036106044 ("vhost-user-scsi: add missing virtqueue_size > param") added that parameter to the new vhost-user-scsi interface but > neglected the existing vhost-scsi interface it was built on. > > Apply the same change to vhost-scsi, so that we can boot a guest with > a device defined. This also avoids crashing a guest when hotplugging > a vhost-scsi device. > > Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> > --- > hw/scsi/vhost-scsi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c > index cd4ab05233..9c1bea8ff3 100644 > --- a/hw/scsi/vhost-scsi.c > +++ b/hw/scsi/vhost-scsi.c > @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { > DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), > DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0), > DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1), > + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size, > + 128), > DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors, > 0xFFFF), > DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128), Yes, this fix is fine. Sorry for breaking it :-/ Rich.
On 12/01/2017 10:16 AM, Paolo Bonzini wrote: > On 01/12/2017 16:15, Eric Farman wrote: >> Commit 5c0919d02066 ("virtio-scsi: Add virtqueue_size parameter allowing >> virtqueue size to be set.") introduced a new parameter to virtio-scsi. >> Later, commit 920036106044 ("vhost-user-scsi: add missing virtqueue_size >> param") added that parameter to the new vhost-user-scsi interface but >> neglected the existing vhost-scsi interface it was built on. >> >> Apply the same change to vhost-scsi, so that we can boot a guest with >> a device defined. This also avoids crashing a guest when hotplugging >> a vhost-scsi device. >> >> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> >> --- >> hw/scsi/vhost-scsi.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c >> index cd4ab05233..9c1bea8ff3 100644 >> --- a/hw/scsi/vhost-scsi.c >> +++ b/hw/scsi/vhost-scsi.c >> @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { >> DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), >> DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0), >> DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1), >> + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size, >> + 128), >> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors, >> 0xFFFF), >> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128), >> > > Queued, thanks. > > Paolo > Wow, super quick, thank you! Seeing conversations today, is this queued for -rc4, or 2.12 and backport to qemu-stable? - Eric
On 4 December 2017 at 17:52, Eric Farman <farman@linux.vnet.ibm.com> wrote: > On 12/01/2017 10:16 AM, Paolo Bonzini wrote: >> >> On 01/12/2017 16:15, Eric Farman wrote: >>> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c >>> index cd4ab05233..9c1bea8ff3 100644 >>> --- a/hw/scsi/vhost-scsi.c >>> +++ b/hw/scsi/vhost-scsi.c >>> @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { >>> DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), >>> DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, >>> 0), >>> DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, >>> 1), >>> + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, >>> conf.virtqueue_size, >>> + 128), >>> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, >>> conf.max_sectors, >>> 0xFFFF), >>> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, >>> conf.cmd_per_lun, 128), > Wow, super quick, thank you! Seeing conversations today, is this queued for > -rc4, or 2.12 and backport to qemu-stable? If people can come to a consensus on whether this is -rc4 material by Tuesday 12:00 GMT that would be helpful. thanks -- PMM
On 12/04/2017 01:05 PM, Peter Maydell wrote: > On 4 December 2017 at 17:52, Eric Farman <farman@linux.vnet.ibm.com> wrote: >> On 12/01/2017 10:16 AM, Paolo Bonzini wrote: >>> >>> On 01/12/2017 16:15, Eric Farman wrote: >>>> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c >>>> index cd4ab05233..9c1bea8ff3 100644 >>>> --- a/hw/scsi/vhost-scsi.c >>>> +++ b/hw/scsi/vhost-scsi.c >>>> @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { >>>> DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), >>>> DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, >>>> 0), >>>> DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, >>>> 1), >>>> + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, >>>> conf.virtqueue_size, >>>> + 128), >>>> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, >>>> conf.max_sectors, >>>> 0xFFFF), >>>> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, >>>> conf.cmd_per_lun, 128), > >> Wow, super quick, thank you! Seeing conversations today, is this queued for >> -rc4, or 2.12 and backport to qemu-stable? > > If people can come to a consensus on whether this is -rc4 material > by Tuesday 12:00 GMT that would be helpful. I'd vote for -rc4, since it's a new guest crash since 2.10. But I also won't complain if it waits for 2.11.1 since I was out of the office during the whole freeze period and no one else noticed it. - Eric
On Mon, Dec 04, 2017 at 03:00:45PM -0500, Eric Farman wrote: > > > On 12/04/2017 01:05 PM, Peter Maydell wrote: > > On 4 December 2017 at 17:52, Eric Farman <farman@linux.vnet.ibm.com> wrote: > > > On 12/01/2017 10:16 AM, Paolo Bonzini wrote: > > > > > > > > On 01/12/2017 16:15, Eric Farman wrote: > > > > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c > > > > > index cd4ab05233..9c1bea8ff3 100644 > > > > > --- a/hw/scsi/vhost-scsi.c > > > > > +++ b/hw/scsi/vhost-scsi.c > > > > > @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { > > > > > DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), > > > > > DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, > > > > > 0), > > > > > DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, > > > > > 1), > > > > > + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, > > > > > conf.virtqueue_size, > > > > > + 128), > > > > > DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, > > > > > conf.max_sectors, > > > > > 0xFFFF), > > > > > DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, > > > > > conf.cmd_per_lun, 128), > > > > > Wow, super quick, thank you! Seeing conversations today, is this queued for > > > -rc4, or 2.12 and backport to qemu-stable? > > > > If people can come to a consensus on whether this is -rc4 material > > by Tuesday 12:00 GMT that would be helpful. > > I'd vote for -rc4, since it's a new guest crash since 2.10. I think so too. > But I also > won't complain if it waits for 2.11.1 since I was out of the office during > the whole freeze period and no one else noticed it. > > - Eric
On 5 December 2017 at 00:59, Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, Dec 04, 2017 at 03:00:45PM -0500, Eric Farman wrote: >> >> >> On 12/04/2017 01:05 PM, Peter Maydell wrote: >> > On 4 December 2017 at 17:52, Eric Farman <farman@linux.vnet.ibm.com> wrote: >> > > On 12/01/2017 10:16 AM, Paolo Bonzini wrote: >> > > > >> > > > On 01/12/2017 16:15, Eric Farman wrote: >> > > > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c >> > > > > index cd4ab05233..9c1bea8ff3 100644 >> > > > > --- a/hw/scsi/vhost-scsi.c >> > > > > +++ b/hw/scsi/vhost-scsi.c >> > > > > @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { >> > > > > DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), >> > > > > DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, >> > > > > 0), >> > > > > DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, >> > > > > 1), >> > > > > + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, >> > > > > conf.virtqueue_size, >> > > > > + 128), >> > > > > DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, >> > > > > conf.max_sectors, >> > > > > 0xFFFF), >> > > > > DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, >> > > > > conf.cmd_per_lun, 128), >> > >> > > Wow, super quick, thank you! Seeing conversations today, is this queued for >> > > -rc4, or 2.12 and backport to qemu-stable? >> > >> > If people can come to a consensus on whether this is -rc4 material >> > by Tuesday 12:00 GMT that would be helpful. >> >> I'd vote for -rc4, since it's a new guest crash since 2.10. > > I think so too. Thanks; spoke also with Paolo on irc, and have pushed this to master for -rc4. -- PMM
On 12/05/2017 07:40 AM, Peter Maydell wrote: > On 5 December 2017 at 00:59, Michael S. Tsirkin <mst@redhat.com> wrote: >> On Mon, Dec 04, 2017 at 03:00:45PM -0500, Eric Farman wrote: >>> >>> >>> On 12/04/2017 01:05 PM, Peter Maydell wrote: >>>> On 4 December 2017 at 17:52, Eric Farman <farman@linux.vnet.ibm.com> wrote: >>>>> On 12/01/2017 10:16 AM, Paolo Bonzini wrote: >>>>>> >>>>>> On 01/12/2017 16:15, Eric Farman wrote: >>>>>>> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c >>>>>>> index cd4ab05233..9c1bea8ff3 100644 >>>>>>> --- a/hw/scsi/vhost-scsi.c >>>>>>> +++ b/hw/scsi/vhost-scsi.c >>>>>>> @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { >>>>>>> DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), >>>>>>> DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, >>>>>>> 0), >>>>>>> DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, >>>>>>> 1), >>>>>>> + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, >>>>>>> conf.virtqueue_size, >>>>>>> + 128), >>>>>>> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, >>>>>>> conf.max_sectors, >>>>>>> 0xFFFF), >>>>>>> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, >>>>>>> conf.cmd_per_lun, 128), >>>> >>>>> Wow, super quick, thank you! Seeing conversations today, is this queued for >>>>> -rc4, or 2.12 and backport to qemu-stable? >>>> >>>> If people can come to a consensus on whether this is -rc4 material >>>> by Tuesday 12:00 GMT that would be helpful. >>> >>> I'd vote for -rc4, since it's a new guest crash since 2.10. >> >> I think so too. > > Thanks; spoke also with Paolo on irc, and have pushed this to master > for -rc4. Cool! I just pulled master/rc4 and tested this scenario, and it works great. Thanks everyone! - Eric
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index cd4ab05233..9c1bea8ff3 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -233,6 +233,8 @@ static Property vhost_scsi_properties[] = { DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn), DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0), DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1), + DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size, + 128), DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors, 0xFFFF), DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128),
Commit 5c0919d02066 ("virtio-scsi: Add virtqueue_size parameter allowing virtqueue size to be set.") introduced a new parameter to virtio-scsi. Later, commit 920036106044 ("vhost-user-scsi: add missing virtqueue_size param") added that parameter to the new vhost-user-scsi interface but neglected the existing vhost-scsi interface it was built on. Apply the same change to vhost-scsi, so that we can boot a guest with a device defined. This also avoids crashing a guest when hotplugging a vhost-scsi device. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> --- hw/scsi/vhost-scsi.c | 2 ++ 1 file changed, 2 insertions(+)