* [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties
@ 2018-06-25 16:42 Kevin Wolf
2018-06-26 8:35 ` Markus Armbruster
0 siblings, 1 reply; 6+ messages in thread
From: Kevin Wolf @ 2018-06-25 16:42 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, mreitz, berrange, pkrempa, armbru, pbonzini, kraxel, qemu-devel
The error handling policy was traditionally set with -drive, but with
-blockdev it is no longer possible to set frontend options. scsi-disk
(and other block devices) have long supported qdev properties to
configure the error handling policy, so let's add these options to
usb-storage as well and just forward them to the internal scsi-disk
instance.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/hw/scsi/scsi.h | 2 ++
hw/scsi/scsi-bus.c | 11 ++++++++++-
hw/usb/dev-storage.c | 2 ++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index e35137ea78..1a7290d563 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -154,6 +154,8 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
int unit, bool removable, int bootindex,
bool share_rw,
+ BlockdevOnError rerror,
+ BlockdevOnError werror,
const char *serial, Error **errp);
void scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
void scsi_legacy_handle_cmdline(void);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 9646743a7d..5905f6bf29 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -226,6 +226,8 @@ static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
int unit, bool removable, int bootindex,
bool share_rw,
+ BlockdevOnError rerror,
+ BlockdevOnError werror,
const char *serial, Error **errp)
{
const char *driver;
@@ -262,6 +264,10 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
object_unparent(OBJECT(dev));
return NULL;
}
+
+ qdev_prop_set_enum(dev, "rerror", rerror);
+ qdev_prop_set_enum(dev, "werror", werror);
+
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err != NULL) {
error_propagate(errp, err);
@@ -285,7 +291,10 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
}
qemu_opts_loc_restore(dinfo->opts);
scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo),
- unit, false, -1, false, NULL, &error_fatal);
+ unit, false, -1, false,
+ BLOCKDEV_ON_ERROR_AUTO,
+ BLOCKDEV_ON_ERROR_AUTO,
+ NULL, &error_fatal);
}
loc_pop(&loc);
}
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index c99398b7f6..cd5551d94f 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -625,6 +625,7 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
&usb_msd_scsi_info_storage, NULL);
scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,
s->conf.bootindex, s->conf.share_rw,
+ s->conf.rerror, s->conf.werror,
dev->serial,
errp);
blk_unref(blk);
@@ -671,6 +672,7 @@ static const VMStateDescription vmstate_usb_msd = {
static Property msd_properties[] = {
DEFINE_BLOCK_PROPERTIES(MSDState, conf),
+ DEFINE_BLOCK_ERROR_PROPERTIES(MSDState, conf),
DEFINE_PROP_BIT("removable", MSDState, removable, 0, false),
DEFINE_PROP_END_OF_LIST(),
};
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties
2018-06-25 16:42 [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties Kevin Wolf
@ 2018-06-26 8:35 ` Markus Armbruster
2018-06-26 8:48 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2018-06-26 8:35 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-block, pkrempa, qemu-devel, armbru, mreitz, kraxel, pbonzini
Kevin Wolf <kwolf@redhat.com> writes:
> The error handling policy was traditionally set with -drive, but with
> -blockdev it is no longer possible to set frontend options. scsi-disk
> (and other block devices) have long supported qdev properties to
> configure the error handling policy, so let's add these options to
> usb-storage as well and just forward them to the internal scsi-disk
> instance.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Let's see whether I remember / figure out how this stuff works.
Not all devices support error handling policy, but scsi-disk does.
The preferred way to configure error handling policy is with qdev
properties rerror, werror. blkconf_apply_backend_options() propagates
them to the associated BlockBackend.
The outmoded way is with -drive parameters rerror, werror.
blockdev_init() propagates them to the BlockBackend.
If you use both ways, qdev properties win.
usb-storage is a hack: it tries to look like a block device, but it's
really a SCSI controller that can serve only a single scsi-disk device,
which it creates automatically.
We want to deprecate -drive. We also want to deprecate usb-storage, but
I guess we're still not ready for that (it's a complicated story).
To deprecate -drive without also deprecating usb-storage, we need to
preserve features.
The outmoded way works with usb-storage: the automatically created
scsi-disk is associated with the BlockBackend that got its error
handling policy from -drive.
The preferred way doesn't, because usb-storage lacks the qdev
properties. This patch adds them.
Kind of sad, but it's either that or deprecating usb-storage.
Patch looks good to me, thus
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Feel free to work whatever parts of my ramblings you find useful into
your commit message.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties
2018-06-26 8:35 ` Markus Armbruster
@ 2018-06-26 8:48 ` Paolo Bonzini
2018-06-26 8:53 ` Daniel P. Berrangé
2018-06-26 11:11 ` Gerd Hoffmann
0 siblings, 2 replies; 6+ messages in thread
From: Paolo Bonzini @ 2018-06-26 8:48 UTC (permalink / raw)
To: Markus Armbruster, Kevin Wolf
Cc: qemu-block, pkrempa, qemu-devel, mreitz, kraxel
On 26/06/2018 10:35, Markus Armbruster wrote:
> We also want to deprecate usb-storage, but
> I guess we're still not ready for that (it's a complicated story).
>
> To deprecate -drive without also deprecating usb-storage, we need to
> preserve features.
Patch looks good to me too, but indeed why can't we deprecate
usb-storage in favor of usb-bot + scsi-disk?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties
2018-06-26 8:48 ` Paolo Bonzini
@ 2018-06-26 8:53 ` Daniel P. Berrangé
2018-06-26 9:14 ` Paolo Bonzini
2018-06-26 11:11 ` Gerd Hoffmann
1 sibling, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-06-26 8:53 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, Kevin Wolf, pkrempa, kraxel, qemu-devel,
qemu-block, mreitz
On Tue, Jun 26, 2018 at 10:48:10AM +0200, Paolo Bonzini wrote:
> On 26/06/2018 10:35, Markus Armbruster wrote:
> > We also want to deprecate usb-storage, but
> > I guess we're still not ready for that (it's a complicated story).
> >
> > To deprecate -drive without also deprecating usb-storage, we need to
> > preserve features.
>
> Patch looks good to me too, but indeed why can't we deprecate
> usb-storage in favor of usb-bot + scsi-disk?
Presumably the migration state of usb-storage is different from the
migration state of usb-bot + scsi-disk, so not a case where libvirt
could silently switch from one to the other. It would need to be an
explicit config change by apps/users.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties
2018-06-26 8:53 ` Daniel P. Berrangé
@ 2018-06-26 9:14 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2018-06-26 9:14 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Markus Armbruster, Kevin Wolf, pkrempa, kraxel, qemu-devel,
qemu-block, mreitz
On 26/06/2018 10:53, Daniel P. Berrangé wrote:
> On Tue, Jun 26, 2018 at 10:48:10AM +0200, Paolo Bonzini wrote:
>> On 26/06/2018 10:35, Markus Armbruster wrote:
>>> We also want to deprecate usb-storage, but
>>> I guess we're still not ready for that (it's a complicated story).
>>>
>>> To deprecate -drive without also deprecating usb-storage, we need to
>>> preserve features.
>>
>> Patch looks good to me too, but indeed why can't we deprecate
>> usb-storage in favor of usb-bot + scsi-disk?
>
> Presumably the migration state of usb-storage is different from the
> migration state of usb-bot + scsi-disk, so not a case where libvirt
> could silently switch from one to the other. It would need to be an
> explicit config change by apps/users.
>From a quick test, migrating
qemu-kvm \
-drive if=none,id=usb,file=$HOME/chipsec.img,snapshot=on \
-usb -device usb-storage,drive=usb -M q35
into
qemu-kvm \
-drive if=none,id=usb,file=$HOME/chipsec.img,snapshot=on \
-usb -device usb-bot -device scsi-disk,drive=usb -M q35
seems to work. Both of them indeed use vmstate_usb_msd for
migration.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties
2018-06-26 8:48 ` Paolo Bonzini
2018-06-26 8:53 ` Daniel P. Berrangé
@ 2018-06-26 11:11 ` Gerd Hoffmann
1 sibling, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2018-06-26 11:11 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, Kevin Wolf, qemu-block, pkrempa, qemu-devel, mreitz
On Tue, Jun 26, 2018 at 10:48:10AM +0200, Paolo Bonzini wrote:
> On 26/06/2018 10:35, Markus Armbruster wrote:
> > We also want to deprecate usb-storage, but
> > I guess we're still not ready for that (it's a complicated story).
> >
> > To deprecate -drive without also deprecating usb-storage, we need to
> > preserve features.
>
> Patch looks good to me too, but indeed why can't we deprecate
> usb-storage in favor of usb-bot + scsi-disk?
I think we can. One blocking issue was hotplug support, but that got fixed a
while ago, and I'm not aware of any other issue ...
cheers,
Gerd
commit b78ecd0998094a8b7e0f14c4888f3a6b525d14ff
Author: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed Jun 15 11:46:58 2016 +0200
usb-bot: hotplug support
This patch marks usb-bot as hot-pluggable device, makes attached
property settable and turns off auto-attach in case the device
was hotplugged.
Hot-plugging a usb-bot device with one or more scsi devices can be
done this way now:
(1) device-add usb-bot,id=foo
(2) device-add scsi-{hd,cd},bus=foo.0,lun=0
(2b) optionally add more devices (luns 0 ... 15).
(3) qom-set foo.attached = true
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1465984019-28963-5-git-send-email-kraxel@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-26 11:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-25 16:42 [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties Kevin Wolf
2018-06-26 8:35 ` Markus Armbruster
2018-06-26 8:48 ` Paolo Bonzini
2018-06-26 8:53 ` Daniel P. Berrangé
2018-06-26 9:14 ` Paolo Bonzini
2018-06-26 11:11 ` Gerd Hoffmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.