* [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
@ 2012-03-22 12:07 Gerd Hoffmann
2012-03-22 12:17 ` Paolo Bonzini
2012-03-22 14:01 ` Andreas Färber
0 siblings, 2 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2012-03-22 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
... to make vmstate id string truely unique with multiple host
controllers, i.e. move from "1/usb-ptr" to "0000:00:01.3/1/usb-ptr"
(usb tabled connected to piix3 uhci).
This obviously breaks migration. To handle this the usb bus
property "full-path" is added. When setting this to false old
behavior is maintained. This way current qemu will be compatible
with old versions when started using '-M pc-$oldversion'.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/pc_piix.c | 28 ++++++++++++++++++++++++++++
hw/usb.h | 5 +++++
hw/usb/bus.c | 18 +++++++++++++++++-
3 files changed, 50 insertions(+), 1 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 3f99f9a..8cb7d5f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = {
.driver = "isa-fdc",
.property = "check_media_rate",
.value = "off",
+ },{
+ .driver = "USB",
+ .property = "full-path",
+ .value = "no",
},
{ /* end of list */ }
},
@@ -405,6 +409,10 @@ static QEMUMachine pc_machine_v0_15 = {
.driver = "isa-fdc",
.property = "check_media_rate",
.value = "off",
+ },{
+ .driver = "USB",
+ .property = "full-path",
+ .value = "no",
},
{ /* end of list */ }
},
@@ -449,6 +457,10 @@ static QEMUMachine pc_machine_v0_14 = {
.driver = "pc-sysfw",
.property = "rom_only",
.value = stringify(1),
+ },{
+ .driver = "USB",
+ .property = "full-path",
+ .value = "no",
},
{ /* end of list */ }
},
@@ -505,6 +517,10 @@ static QEMUMachine pc_machine_v0_13 = {
.driver = "pc-sysfw",
.property = "rom_only",
.value = stringify(1),
+ },{
+ .driver = "USB",
+ .property = "full-path",
+ .value = "no",
},
{ /* end of list */ }
},
@@ -565,6 +581,10 @@ static QEMUMachine pc_machine_v0_12 = {
.driver = "pc-sysfw",
.property = "rom_only",
.value = stringify(1),
+ },{
+ .driver = "USB",
+ .property = "full-path",
+ .value = "no",
},
{ /* end of list */ }
}
@@ -633,6 +653,10 @@ static QEMUMachine pc_machine_v0_11 = {
.driver = "pc-sysfw",
.property = "rom_only",
.value = stringify(1),
+ },{
+ .driver = "USB",
+ .property = "full-path",
+ .value = "no",
},
{ /* end of list */ }
}
@@ -713,6 +737,10 @@ static QEMUMachine pc_machine_v0_10 = {
.driver = "pc-sysfw",
.property = "rom_only",
.value = stringify(1),
+ },{
+ .driver = "USB",
+ .property = "full-path",
+ .value = "no",
},
{ /* end of list */ }
},
diff --git a/hw/usb.h b/hw/usb.h
index e95085f..ae7ccda 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -182,12 +182,17 @@ struct USBEndpoint {
QTAILQ_HEAD(, USBPacket) queue;
};
+enum USBDeviceFlags {
+ USB_DEV_FLAG_FULL_PATH,
+};
+
/* definition of a USB device */
struct USBDevice {
DeviceState qdev;
USBPort *port;
char *port_path;
void *opaque;
+ uint32_t flags;
/* Actual connected speed */
int speed;
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index d3f8358..f1e567b 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -19,6 +19,8 @@ static struct BusInfo usb_bus_info = {
.get_fw_dev_path = usb_get_fw_dev_path,
.props = (Property[]) {
DEFINE_PROP_STRING("port", USBDevice, port_path),
+ DEFINE_PROP_BIT("full-path", USBDevice, flags,
+ USB_DEV_FLAG_FULL_PATH, true),
DEFINE_PROP_END_OF_LIST()
},
};
@@ -460,7 +462,21 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
static char *usb_get_dev_path(DeviceState *qdev)
{
USBDevice *dev = USB_DEVICE(qdev);
- return g_strdup(dev->port->path);
+ DeviceState *hcd = qdev->parent_bus->parent;
+ char *id = NULL;
+
+ if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) &&
+ hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) {
+ id = hcd->parent_bus->info->get_dev_path(hcd);
+ }
+ if (id) {
+ int len = strlen(id)+strlen(dev->port->path)+2;
+ char *ret = g_malloc(len);
+ snprintf(ret, len, "%s/%s", id, dev->port->path);
+ return ret;
+ } else {
+ return g_strdup(dev->port->path);
+ }
}
static char *usb_get_fw_dev_path(DeviceState *qdev)
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
2012-03-22 12:07 [Qemu-devel] [PATCH] usb/vmstate: add parent dev path Gerd Hoffmann
@ 2012-03-22 12:17 ` Paolo Bonzini
2012-03-22 14:01 ` Andreas Färber
1 sibling, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2012-03-22 12:17 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
Il 22/03/2012 13:07, Gerd Hoffmann ha scritto:
> @@ -460,7 +462,21 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
> static char *usb_get_dev_path(DeviceState *qdev)
> {
> USBDevice *dev = USB_DEVICE(qdev);
> - return g_strdup(dev->port->path);
> + DeviceState *hcd = qdev->parent_bus->parent;
> + char *id = NULL;
> +
> + if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) &&
> + hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) {
> + id = hcd->parent_bus->info->get_dev_path(hcd);
> + }
> + if (id) {
> + int len = strlen(id)+strlen(dev->port->path)+2;
> + char *ret = g_malloc(len);
> + snprintf(ret, len, "%s/%s", id, dev->port->path);
You can use g_strdup_printf here, also this is leaking id (I have to fix
it for SCSI too).
> + return ret;
> + } else {
> + return g_strdup(dev->port->path);
> + }
> }
>
> static char *usb_get_fw_dev_path(DeviceState *qdev)
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
2012-03-22 12:07 [Qemu-devel] [PATCH] usb/vmstate: add parent dev path Gerd Hoffmann
2012-03-22 12:17 ` Paolo Bonzini
@ 2012-03-22 14:01 ` Andreas Färber
2012-03-22 14:37 ` Gerd Hoffmann
1 sibling, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2012-03-22 14:01 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Paolo Bonzini, qemu-devel, Anthony Liguori
Am 22.03.2012 13:07, schrieb Gerd Hoffmann:
> ... to make vmstate id string truely unique with multiple host
> controllers, i.e. move from "1/usb-ptr" to "0000:00:01.3/1/usb-ptr"
> (usb tabled connected to piix3 uhci).
>
> This obviously breaks migration. To handle this the usb bus
> property "full-path" is added. When setting this to false old
> behavior is maintained. This way current qemu will be compatible
> with old versions when started using '-M pc-$oldversion'.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/pc_piix.c | 28 ++++++++++++++++++++++++++++
> hw/usb.h | 5 +++++
> hw/usb/bus.c | 18 +++++++++++++++++-
> 3 files changed, 50 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 3f99f9a..8cb7d5f 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = {
> .driver = "isa-fdc",
> .property = "check_media_rate",
> .value = "off",
> + },{
> + .driver = "USB",
> + .property = "full-path",
> + .value = "no",
This touches on our "favorite" bit/bool topic again. While I agree that
"no" makes sense for a property of that name, the current code still
expects "on" and "off". In particular, "yes" would not work as expected.
We should either merge support for yes/no or use "off" here.
I had a patch for the former but didn't resubmit after the QOM merge
since I thought we would want to move this from qdev to Object first.
Andreas
> },
> { /* end of list */ }
> },
> @@ -405,6 +409,10 @@ static QEMUMachine pc_machine_v0_15 = {
> .driver = "isa-fdc",
> .property = "check_media_rate",
> .value = "off",
> + },{
> + .driver = "USB",
> + .property = "full-path",
> + .value = "no",
> },
> { /* end of list */ }
> },
> @@ -449,6 +457,10 @@ static QEMUMachine pc_machine_v0_14 = {
> .driver = "pc-sysfw",
> .property = "rom_only",
> .value = stringify(1),
> + },{
> + .driver = "USB",
> + .property = "full-path",
> + .value = "no",
> },
> { /* end of list */ }
> },
> @@ -505,6 +517,10 @@ static QEMUMachine pc_machine_v0_13 = {
> .driver = "pc-sysfw",
> .property = "rom_only",
> .value = stringify(1),
> + },{
> + .driver = "USB",
> + .property = "full-path",
> + .value = "no",
> },
> { /* end of list */ }
> },
> @@ -565,6 +581,10 @@ static QEMUMachine pc_machine_v0_12 = {
> .driver = "pc-sysfw",
> .property = "rom_only",
> .value = stringify(1),
> + },{
> + .driver = "USB",
> + .property = "full-path",
> + .value = "no",
> },
> { /* end of list */ }
> }
> @@ -633,6 +653,10 @@ static QEMUMachine pc_machine_v0_11 = {
> .driver = "pc-sysfw",
> .property = "rom_only",
> .value = stringify(1),
> + },{
> + .driver = "USB",
> + .property = "full-path",
> + .value = "no",
> },
> { /* end of list */ }
> }
> @@ -713,6 +737,10 @@ static QEMUMachine pc_machine_v0_10 = {
> .driver = "pc-sysfw",
> .property = "rom_only",
> .value = stringify(1),
> + },{
> + .driver = "USB",
> + .property = "full-path",
> + .value = "no",
> },
> { /* end of list */ }
> },
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index d3f8358..f1e567b 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -19,6 +19,8 @@ static struct BusInfo usb_bus_info = {
> .get_fw_dev_path = usb_get_fw_dev_path,
> .props = (Property[]) {
> DEFINE_PROP_STRING("port", USBDevice, port_path),
> + DEFINE_PROP_BIT("full-path", USBDevice, flags,
> + USB_DEV_FLAG_FULL_PATH, true),
> DEFINE_PROP_END_OF_LIST()
> },
> };
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
2012-03-22 14:01 ` Andreas Färber
@ 2012-03-22 14:37 ` Gerd Hoffmann
2012-03-22 15:12 ` Andreas Färber
0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2012-03-22 14:37 UTC (permalink / raw)
To: Andreas Färber; +Cc: Paolo Bonzini, qemu-devel, Anthony Liguori
Hi,
>> + .driver = "USB",
>> + .property = "full-path",
>> + .value = "no",
>
> This touches on our "favorite" bit/bool topic again. While I agree that
> "no" makes sense for a property of that name, the current code still
> expects "on" and "off". In particular, "yes" would not work as expected.
"no" *does* work as expected though, at least it survived my tests,
thats why I didn't notice.
I can s/no/off/ for consistency, no problem. Merging your yes/no patch
is fine with me too.
cheers,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
2012-03-22 14:37 ` Gerd Hoffmann
@ 2012-03-22 15:12 ` Andreas Färber
2012-03-28 22:08 ` Michael Roth
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2012-03-22 15:12 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Paolo Bonzini, qemu-devel, Anthony Liguori, Michael Roth
Am 22.03.2012 15:37, schrieb Gerd Hoffmann:
>>> + .driver = "USB",
>>> + .property = "full-path",
>>> + .value = "no",
>>
>> This touches on our "favorite" bit/bool topic again. While I agree that
>> "no" makes sense for a property of that name, the current code still
>> expects "on" and "off". In particular, "yes" would not work as expected.
>
> "no" *does* work as expected though, at least it survived my tests,
> thats why I didn't notice.
>
> I can s/no/off/ for consistency, no problem. Merging your yes/no patch
> is fine with me too.
I stand corrected: Michael Roth's conversion of qdev properties to use
visitors implicitly changed the parsing logic to cover yes/no and
true/false as well. That's great news!
So no change of .value is needed here. The only thing to note is that
print_bit() will output the value as on/off.
Sorry for the confusion,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
2012-03-22 15:12 ` Andreas Färber
@ 2012-03-28 22:08 ` Michael Roth
0 siblings, 0 replies; 6+ messages in thread
From: Michael Roth @ 2012-03-28 22:08 UTC (permalink / raw)
To: Andreas Färber
Cc: Paolo Bonzini, Gerd Hoffmann, Anthony Liguori, qemu-devel
On Thu, Mar 22, 2012 at 04:12:12PM +0100, Andreas Färber wrote:
> Am 22.03.2012 15:37, schrieb Gerd Hoffmann:
> >>> + .driver = "USB",
> >>> + .property = "full-path",
> >>> + .value = "no",
> >>
> >> This touches on our "favorite" bit/bool topic again. While I agree that
> >> "no" makes sense for a property of that name, the current code still
> >> expects "on" and "off". In particular, "yes" would not work as expected.
> >
> > "no" *does* work as expected though, at least it survived my tests,
> > thats why I didn't notice.
> >
> > I can s/no/off/ for consistency, no problem. Merging your yes/no patch
> > is fine with me too.
>
> I stand corrected: Michael Roth's conversion of qdev properties to use
> visitors implicitly changed the parsing logic to cover yes/no and
> true/false as well. That's great news!
I believe Paolo did that conversion, but agreed :)
>
> So no change of .value is needed here. The only thing to note is that
> print_bit() will output the value as on/off.
>
> Sorry for the confusion,
>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-28 23:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-22 12:07 [Qemu-devel] [PATCH] usb/vmstate: add parent dev path Gerd Hoffmann
2012-03-22 12:17 ` Paolo Bonzini
2012-03-22 14:01 ` Andreas Färber
2012-03-22 14:37 ` Gerd Hoffmann
2012-03-22 15:12 ` Andreas Färber
2012-03-28 22:08 ` Michael Roth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).