Re: [Qemu-devel] [PATCH] qga: Fix possible freed memory accessing
zhanghailiang writes: > If readdir_r fails, error_setg_errno will reference the freed > pointer *dirpath*. > > Signed-off-by: zhanghailiang Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2] slirp: udp: fix NULL pointer dereference because of uninitialized socket
On Thu, Sep 18, 2014 at 08:35:37AM +0200, Petr Matousek wrote:
> When guest sends udp packet with source port and source addr 0,
> uninitialized socket is picked up when looking for matching and already
> created udp sockets, and later passed to sosendto() where NULL pointer
> dereference is hit during so->slirp->vnetwork_mask.s_addr access.
>
> Fix this by checking that the socket is not just a socket stub.
>
> This is CVE-2014-3640.
>
> Signed-off-by: Petr Matousek
> Reported-by: Xavier Mehrenberger
> Reported-by: Stephane Duverger
Reviewed-by: Michael S. Tsirkin
> ---
> v1 -> v2
> * change the check so that it's consistent with the rest of the code
>
> slirp/udp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/slirp/udp.c b/slirp/udp.c
> index 8cc6cb6..f77e00f 100644
> --- a/slirp/udp.c
> +++ b/slirp/udp.c
> @@ -152,7 +152,7 @@ udp_input(register struct mbuf *m, int iphlen)
>* Locate pcb for datagram.
>*/
> so = slirp->udp_last_so;
> - if (so->so_lport != uh->uh_sport ||
> + if (so == &slirp->udb || so->so_lport != uh->uh_sport ||
> so->so_laddr.s_addr != ip->ip_src.s_addr) {
> struct socket *tmp;
>
> --
> 1.9.3
Re: [Qemu-devel] [PATCH v3 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo()
Benoît Canet writes:
> On Tue, Sep 16, 2014 at 08:12:15PM +0200, Markus Armbruster wrote:
Restoring context...
@@ -252,14 +253,16 @@ static int milkymist_memcard_init(SysBusDevice *dev)
{
MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
DriveInfo *dinfo;
+BlockDriverState *bs;
dinfo = drive_get_next(IF_SD);
-s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false);
+bs = dinfo ? blk_bs(blk_by_legacy_dinfo(dinfo)) : NULL;
+s->card = sd_init(bs, false);
if (s->card == NULL) {
return -1;
}
>> -s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
>> +s->enabled = bs && bdrv_is_inserted(bs);
memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,
"milkymist-memcard", R_MAX * 4);
>
> This is not so mechanical but seems correct anyway.
The mechanical change would be
s->enabled = dinfo ? bdrv_is_inserted(blk_by_legacy_dinfo(dinfo)) : 0;
My actual change exploits that the value of blk_by_legacy_dinfo(dinfo)
is already available in bs, and that bs is null if and only if dinfo is
null.
> Reviewed-by: Benoit Canet
Thanks!
Re: [Qemu-devel] [PATCH v3 13/23] virtio-blk: Rename VirtIOBlkConf variables to conf
Benoît Canet writes:
>> -if (size % dev->blk.conf.logical_block_size) {
>> +if (size % dev->conf.conf.logical_block_size) {
>
> This look strange (conf.conf)
Yeah, it does. I picked conf anyway, for consistency with the existing
similar members of sibling structs VirtIOSCSICommon and VirtIORNG.
> Anyway this seems correct.
>
> Reviewed-by: Benoit Canet
Thanks!
Re: [Qemu-devel] [PATCH] usb-storage: fix possible memory leak and missing error message
writes:
> From: Gonglei
>
> When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
> be not NULL, which will casue memory leak and missing error message.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-storage.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 5c51ac0..cabd053 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -599,7 +599,6 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> {
> MSDState *s = DO_UPCAST(MSDState, dev, dev);
> BlockDriverState *bs = s->conf.bs;
> -SCSIDevice *scsi_dev;
> Error *err = NULL;
>
> if (!bs) {
> @@ -625,10 +624,12 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> usb_desc_init(dev);
> scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
> &usb_msd_scsi_info_storage, NULL);
> -scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> - s->conf.bootindex, dev->serial,
> - &err);
> -if (!scsi_dev) {
> +scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> + s->conf.bootindex, dev->serial,
> + &err);
> +if (err) {
> +error_report("%s", error_get_pretty(err));
> +error_free(err);
> return -1;
> }
> s->bus.qbus.allow_hotplug = 0;
I'd keep the original condition and just fix the error message loss /
error object leak:
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae4efcb..f731b0a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
s->conf.bootindex, dev->serial,
&err);
if (!scsi_dev) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
return -1;
}
s->bus.qbus.allow_hotplug = 0;
Partly because it makes the fix more obvious, partly because I prefer
checking the function value when it is available. Matter of taste.
scsi_hot_add() in pci-hotplug-old.c also loses the error message. Would
you care to fix that, too?
[Qemu-devel] vhost-blk didn't work on Windows Guest
Hi,
I want to use vhost-blk on Windows 8 guest, but vhost-blk didn't work.
When Windows 8 guest was boot-up, vhost-blk couldn't start and Windows 8
guest is hang on boot sequence.
I checked vhost-blk (qemu, linux kernel module) and I found out the msi-x
is not enabled when Windows guest is booting.
So, I want to know is, Can I use vhost-blk with Windows guest?
and Does Virtio-blk driver for Windows 8 supports msi-x?
I use that source code for using vhost-blk.
https://github.com/asias/qemu.git (blk.vhost-blk)
https://github.com/asias/linux.git (blk.vhost-blk)
This is my qemu command line,
-drive id=drive0,if=none,media=disk,vhost=off,file=Windows8.qcow2
-device virtio-blk-pci,drive=drive0,bootindex=1
-drive if=virtio,media=disk,cache=none,aio=native,vhost=on,file=/dev/sdc
And fail on this point,
(in qemu source, hw/block/virtio-blk.c)
81 if (!vhost_dev_query(&vb->dev, vdev)) {
82 return -ENOTSUP;
83 }
Thanks,
Re: [Qemu-devel] [PATCH] block: vhdx - fix reading beyond pointer during image creation
Markus Armbruster writes: > Jeff Cody writes: > >> On Wed, Sep 17, 2014 at 08:33:10AM +0200, Markus Armbruster wrote: >>> Jeff Cody writes: >>> >>> > In vhdx_create_metadata(), we allocate 40 bytes to entry_buffer for >>> > the various metadata table entries. However, we write out 64kB from >>> > that buffer into the new file. Only write out the correct 40 bytes. >>> > >>> > Signed-off-by: Jeff Cody >>> > --- >>> > block/vhdx.c | 16 >>> > 1 file changed, 8 insertions(+), 8 deletions(-) >>> > >>> > diff --git a/block/vhdx.c b/block/vhdx.c >>> > index 796b7bd..b52ec32 100644 >>> > --- a/block/vhdx.c >>> > +++ b/block/vhdx.c >>> > @@ -1407,6 +1407,12 @@ exit: >>> > return ret; >>> > } >>> > >>> > +#define VHDX_METADATA_ENTRY_BUFFER_SIZE \ >>> > +(sizeof(VHDXFileParameters) >>> > +\ >>> > + sizeof(VHDXVirtualDiskSize) >>> > +\ >>> > + sizeof(VHDXPage83Data) >>> > +\ >>> > + >>> > sizeof(VHDXVirtualDiskLogicalSectorSize) +\ >>> > + >>> > sizeof(VHDXVirtualDiskPhysicalSectorSize)) >>> >>> Long lines, caused by excessive indentation. Emacs suggests >>> >>> #define VHDX_METADATA_ENTRY_BUFFER_SIZE \ >>> (sizeof(VHDXFileParameters) + \ >>> sizeof(VHDXVirtualDiskSize) + \ >>> sizeof(VHDXPage83Data) + \ >>> sizeof(VHDXVirtualDiskLogicalSectorSize) + \ >>> sizeof(VHDXVirtualDiskPhysicalSectorSize)) >>> >> >> So, I was getting ready to respin this, but double checked the patch - >> it shows the lines ending on column 80 (as intended), and >> checkpatch.pl had no issue with it. Did you accidentally (or >> intentionally!) count the leading '+' of the patch itself? > > I didn't count anything, I trusted my eyes, which screamed "ugly!" :) Forgot to say: since checkpatch is happy, I guess this is a technically a matter of taste, so Reviewed-by: Markus Armbruster Just in case you actually *like* hanging your code right off the right margin of the window. *Shudder* ;-P
Re: [Qemu-devel] [PATCH] block: Validate node-name
Kevin Wolf writes:
> Am 17.09.2014 um 13:49 hat Benoît Canet geschrieben:
>>
>>
>> > +int qemu_opts_id_wellformed(const char *id)
>>
>> This return 0 and 1 as a bool.
>> Could we make the function return bool in the same series ?
>
> I considered the change (as you probably saw, the new block.c function
> returns a bool), but then thought it wasn't important enough.
>
> In any case, that would be something for a separate patch. If you think
> it's important, I can send one.
>
>> I wonder what are the possible interferences between !strchr("-._", id[i])
>> and Jeff's node name auto naming series.
>
> We might need to update the code then, but it would actually be a good
> reason why auto-naming wouldn't hurt if it uses characters that you
> can't use manually.
I'm afraid this is something we should ponder in a wider context, not
just BDS names. Ties to other users of QemuOpts IDs, such as qdev, and
to how QOM lets users refer to objects.
[Qemu-devel] [PATCH v1 0/3] monitor: add peripheral device del completion support
After inputting device_del command in monitor, we expect to list all hotpluggable devices automatically by pressing tab key. This patchset provides the function to list all peripheral devices such as memory devices. Zhu Guihua (3): qom: add function to get opaque property in ObjectProperty qom: export object_property_is_child() monitor: add del completion for peripheral device include/qom/object.h | 2 ++ monitor.c| 24 qom/object.c | 11 ++- 3 files changed, 36 insertions(+), 1 deletion(-) -- 1.9.3
Re: [Qemu-devel] [PATCH] usb-storage: fix possible memory leak and missing error message
> From: Markus Armbruster [mailto:arm...@redhat.com]
> Sent: Thursday, September 18, 2014 3:38 PM
> Subject: Re: [Qemu-devel] [PATCH] usb-storage: fix possible memory leak and
> missing error message
>
> writes:
>
> > From: Gonglei
> >
> > When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
> > be not NULL, which will casue memory leak and missing error message.
> >
> > Signed-off-by: Gonglei
> > ---
> > hw/usb/dev-storage.c | 11 ++-
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> > index 5c51ac0..cabd053 100644
> > --- a/hw/usb/dev-storage.c
> > +++ b/hw/usb/dev-storage.c
> > @@ -599,7 +599,6 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> > {
> > MSDState *s = DO_UPCAST(MSDState, dev, dev);
> > BlockDriverState *bs = s->conf.bs;
> > -SCSIDevice *scsi_dev;
> > Error *err = NULL;
> >
> > if (!bs) {
> > @@ -625,10 +624,12 @@ static int usb_msd_initfn_storage(USBDevice
> *dev)
> > usb_desc_init(dev);
> > scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
> > &usb_msd_scsi_info_storage, NULL);
> > -scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> > - s->conf.bootindex,
> dev->serial,
> > - &err);
> > -if (!scsi_dev) {
> > +scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> > + s->conf.bootindex, dev->serial,
> > + &err);
> > +if (err) {
> > +error_report("%s", error_get_pretty(err));
> > +error_free(err);
> > return -1;
> > }
> > s->bus.qbus.allow_hotplug = 0;
>
> I'd keep the original condition and just fix the error message loss /
> error object leak:
>
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index ae4efcb..f731b0a 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> s->conf.bootindex,
> dev->serial,
> &err);
> if (!scsi_dev) {
> +error_report("%s", error_get_pretty(err));
> +error_free(err);
> return -1;
> }
> s->bus.qbus.allow_hotplug = 0;
>
> Partly because it makes the fix more obvious, partly because I prefer
> checking the function value when it is available. Matter of taste.
>
Hmm..
The main problem is I'm afraid the scenario when scsi_dev
is NULL the err is NULL too in someday. I have fixed a similar issue
and you have reviewed. :)
> scsi_hot_add() in pci-hotplug-old.c also loses the error message. Would
> you care to fix that, too?
I have noticed that, but because scsi_hot_add() pass NULL to
scsi_bus_legacy_add_drive(), so I let it pass. As you remainder,
I will post another patch to fix it. Thanks!
Best regards,
-Gonglei
[Qemu-devel] [PATCH v1 2/3] qom: export object_property_is_child()
Export object_property_is_child() to let it be invoked in other places.
Signed-off-by: Zhu Guihua
---
include/qom/object.h | 1 +
qom/object.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 5d55889..8f27b7c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1302,4 +1302,5 @@ Object *container_get(Object *root, const char *path);
void *object_property_get_opaque(ObjectProperty *prop, Error **errp);
+bool object_property_is_child(ObjectProperty *prop);
#endif
diff --git a/qom/object.c b/qom/object.c
index 00a25e0..8de2599 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -351,7 +351,7 @@ void object_initialize(void *data, size_t size, const char
*typename)
object_initialize_with_type(data, size, type);
}
-static inline bool object_property_is_child(ObjectProperty *prop)
+bool object_property_is_child(ObjectProperty *prop)
{
return strstart(prop->type, "child<", NULL);
}
--
1.9.3
[Qemu-devel] [PATCH v1 1/3] qom: add function to get opaque property in ObjectProperty
Add object_property_get_opaque() to get opaque property in ObjectProperty.
Signed-off-by: Zhu Guihua
---
include/qom/object.h | 1 +
qom/object.c | 9 +
2 files changed, 10 insertions(+)
diff --git a/include/qom/object.h b/include/qom/object.h
index 8a05a81..5d55889 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1300,5 +1300,6 @@ int object_child_foreach(Object *obj, int (*fn)(Object
*child, void *opaque),
*/
Object *container_get(Object *root, const char *path);
+void *object_property_get_opaque(ObjectProperty *prop, Error **errp);
#endif
diff --git a/qom/object.c b/qom/object.c
index da0919a..00a25e0 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -356,6 +356,15 @@ static inline bool object_property_is_child(ObjectProperty
*prop)
return strstart(prop->type, "child<", NULL);
}
+void *object_property_get_opaque(ObjectProperty *prop, Error **errp)
+{
+if (prop == NULL) {
+return NULL;
+}
+
+return prop->opaque;
+}
+
static void object_property_del_all(Object *obj)
{
while (!QTAILQ_EMPTY(&obj->properties)) {
--
1.9.3
[Qemu-devel] [PATCH v1 3/3] monitor: add del completion for peripheral device
Add peripheral_device_del_completion() to let peripheral device del completion
be possible.
Signed-off-by: Zhu Guihua
---
monitor.c | 24
1 file changed, 24 insertions(+)
diff --git a/monitor.c b/monitor.c
index 667efb7..c0e00e4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4351,6 +4351,29 @@ static void device_del_bus_completion(ReadLineState *rs,
BusState *bus,
}
}
+static void peripheral_device_del_completion(ReadLineState *rs,
+ const char *str, size_t len)
+{
+Object *peripheral;
+DeviceState *dev = NULL;
+ObjectProperty *prop;
+
+peripheral = object_resolve_path("/machine/peripheral/", NULL);
+
+if (peripheral == NULL) {
+return;
+}
+
+QTAILQ_FOREACH(prop, &peripheral->properties, node) {
+if (object_property_is_child(prop)) {
+dev = DEVICE(object_property_get_opaque(prop, NULL));
+if (dev->id && !strncmp(str, dev->id, len)) {
+readline_add_completion(rs, dev->id);
+}
+}
+}
+}
+
void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str)
{
size_t len;
@@ -4424,6 +4447,7 @@ void device_del_completion(ReadLineState *rs, int
nb_args, const char *str)
len = strlen(str);
readline_set_completion_index(rs, len);
device_del_bus_completion(rs, sysbus_get_default(), str, len);
+peripheral_device_del_completion(rs, str, len);
}
void object_del_completion(ReadLineState *rs, int nb_args, const char *str)
--
1.9.3
Re: [Qemu-devel] [PATCH v4] Add HMP command "info memory-devices"
Hi,
Could anyone help to review this patch?
If there was no problem, could help to merge it?
thanks!
zhu
On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote:
> Provides HMP equivalent of QMP query-memory-devices command.
>
> Signed-off-by: Zhu Guihua
> ---
>
> Changes since v3:
> - optimize the time to print memory devices' information.
> - change output format of di->addr and di->size.
>
> Changes since v2:
> - print address in hex.
> - change the loop control from while to for.
> - modify some variables' name.
> - optimize the time to print memory devices' kind.
>
> Changes since v1:
> - fix bug that accessing info->dimm when MemoryDeviceInfo is not PCDIMMDevice.
> - use enum to replace "dimm", and lookup typename in
> MemoryDeviceInfoKind_lookup[] instead of opencodding it.
>
> hmp-commands.hx | 2 ++
> hmp.c | 38 ++
> hmp.h | 1 +
> monitor.c | 7 +++
> 4 files changed, 48 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f859f8d..0b1a4f7 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1778,6 +1778,8 @@ show qdev device model list
> show roms
> @item info tpm
> show the TPM device
> +@item info memory-devices
> +show the memory devices
> @end table
> ETEXI
>
> diff --git a/hmp.c b/hmp.c
> index 40a90da..feefeb4 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
>
> qapi_free_MemdevList(memdev_list);
> }
> +
> +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
> +{
> +Error *err = NULL;
> +MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err);
> +MemoryDeviceInfoList *info;
> +MemoryDeviceInfo *value;
> +PCDIMMDeviceInfo *di;
> +
> +for (info = info_list; info; info = info->next) {
> +value = info->value;
> +
> +if (value) {
> +switch (value->kind) {
> +case MEMORY_DEVICE_INFO_KIND_DIMM:
> +di = value->dimm;
> +
> +monitor_printf(mon, "Memory device [%s]: %s\n",
> + MemoryDeviceInfoKind_lookup[value->kind],
> + di->id);
> +monitor_printf(mon, " addr: 0x%" PRIx64 "\n", di->addr);
> +monitor_printf(mon, " slot: %" PRId64 "\n", di->slot);
> +monitor_printf(mon, " node: %" PRId64 "\n", di->node);
> +monitor_printf(mon, " size: %" PRIu64 "\n", di->size);
> +monitor_printf(mon, " memdev: %s\n", di->memdev);
> +monitor_printf(mon, " hotplugged: %s\n",
> + di->hotplugged ? "true" : "false");
> +monitor_printf(mon, " hotpluggable: %s\n",
> + di->hotpluggable ? "true" : "false");
> +break;
> +default:
> +break;
> +}
> +}
> +}
> +
> +qapi_free_MemoryDeviceInfoList(info_list);
> +}
> diff --git a/hmp.h b/hmp.h
> index 4fd3c4a..4bb5dca 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
> void hmp_object_add(Monitor *mon, const QDict *qdict);
> void hmp_object_del(Monitor *mon, const QDict *qdict);
> void hmp_info_memdev(Monitor *mon, const QDict *qdict);
> +void hmp_info_memory_devices(Monitor *mon, const QDict *qdict);
> void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
> void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
> void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
> diff --git a/monitor.c b/monitor.c
> index 34cee74..fe88e0d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = {
> .mhandler.cmd = hmp_info_memdev,
> },
> {
> +.name = "memory-devices",
> +.args_type = "",
> +.params = "",
> +.help = "show memory devices",
> +.mhandler.cmd = hmp_info_memory_devices,
> +},
> +{
> .name = NULL,
> },
> };
Re: [Qemu-devel] [PATCH] usb-storage: fix possible memory leak and missing error message
"Gonglei (Arei)" writes:
>> From: Markus Armbruster [mailto:arm...@redhat.com]
>> Sent: Thursday, September 18, 2014 3:38 PM
>> Subject: Re: [Qemu-devel] [PATCH] usb-storage: fix possible memory leak and
>> missing error message
>>
>> writes:
>>
>> > From: Gonglei
>> >
>> > When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
>> > be not NULL, which will casue memory leak and missing error message.
>> >
>> > Signed-off-by: Gonglei
>> > ---
>> > hw/usb/dev-storage.c | 11 ++-
>> > 1 file changed, 6 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
>> > index 5c51ac0..cabd053 100644
>> > --- a/hw/usb/dev-storage.c
>> > +++ b/hw/usb/dev-storage.c
>> > @@ -599,7 +599,6 @@ static int usb_msd_initfn_storage(USBDevice *dev)
>> > {
>> > MSDState *s = DO_UPCAST(MSDState, dev, dev);
>> > BlockDriverState *bs = s->conf.bs;
>> > -SCSIDevice *scsi_dev;
>> > Error *err = NULL;
>> >
>> > if (!bs) {
>> > @@ -625,10 +624,12 @@ static int usb_msd_initfn_storage(USBDevice *dev)
>> > usb_desc_init(dev);
>> > scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
>> > &usb_msd_scsi_info_storage, NULL);
>> > -scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
>> > - s->conf.bootindex, dev->serial,
>> > - &err);
>> > -if (!scsi_dev) {
>> > +scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
>> > + s->conf.bootindex, dev->serial,
>> > + &err);
>> > +if (err) {
>> > +error_report("%s", error_get_pretty(err));
>> > +error_free(err);
>> > return -1;
>> > }
>> > s->bus.qbus.allow_hotplug = 0;
>>
>> I'd keep the original condition and just fix the error message loss /
>> error object leak:
>>
>> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
>> index ae4efcb..f731b0a 100644
>> --- a/hw/usb/dev-storage.c
>> +++ b/hw/usb/dev-storage.c
>> @@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
>> s->conf.bootindex,
>> dev->serial,
>> &err);
>> if (!scsi_dev) {
>> +error_report("%s", error_get_pretty(err));
>> +error_free(err);
>> return -1;
>> }
>> s->bus.qbus.allow_hotplug = 0;
>>
>> Partly because it makes the fix more obvious, partly because I prefer
>> checking the function value when it is available. Matter of taste.
>>
> Hmm..
>
> The main problem is I'm afraid the scenario when scsi_dev
> is NULL the err is NULL too in someday. I have fixed a similar issue
> and you have reviewed. :)
Short answer: doesn't help, and you shouldn't be afraid anyway.
Now the long answer :)
Basic rules about returning errors through Error **errp parameters:
1. The errp argument may be null.
2. If it isn't, then it points to null.
3. The function either succeeds or fails.
4. If the function succeeds, it doesn't touch *errp.
5. If the function fails, and
a. if errp isn't null, it creates an Error object and returns it in
*errp. The caller is responsible for freeing it.
b. if errp is null, no Error object is created.
Corollary: when you pass a non-null errp argument, *errp is null exactly
on success.
Rule 3 connects the Error contract with what else the function's
supposed to do on success and on failure. In particular with the return
value.
Common case: a function returns a pointer to something on success, null
pointer on failure. When you pass a non-null errp, argument, *errp is
null exactly when the function value is non-null. In other words,
either the value or *errp is null.
To detect errors, you can either check the function value, or your errp
argument.
If you detect an error, you can either handle it locally, or propagate
it to your caller.
Example: consider a function new_foo() returning a newly allocated Foo
on success, null on error. Any error we want to propagate.
Checking the errp argument:
Error *local_err = NULL;
foop = new_foo(arg, &local_err);
if (local_err) {
error_propagate(errp, local_err);
// rely on postcondition !foop (else we'd leak it here)
return;
}
Checking the function value:
Error *local_err = NULL;
foop = new_foo(arg, &local_err);
if (!foop) {
error_propagate(errp, local_err);
return;
}
// rely on postcondition !local_err (else, we'd leak it here)
No matter which of the two you check, you rely on new_foo()'s
postcondition "either value or *errp is null".
Therefore, switching from one to the other does not make the code more
robust.
Checking the function value can be simplified to just
foop = new_foo(arg, errp);
if (!foop) {
return;
}
I use this whenever possible, because it's simpler, and less clutter.
What
[Qemu-devel] [PATCH] pci-hotplug-old: avoid lossing error message
From: Gonglei
When scsi_bus_legacy_add_drive() produce error,
we will loss error message. Using error_report
report it.
Cc: Markus Armbruster
Signed-off-by: Gonglei
---
hw/pci/pci-hotplug-old.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index cf2caeb..ed85f24 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -107,6 +107,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
{
SCSIBus *scsibus;
SCSIDevice *scsidev;
+Error *local_err = NULL;
scsibus = (SCSIBus *)
object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
@@ -127,8 +128,10 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
dinfo->bus = scsibus->busnr;
scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
-false, -1, NULL, NULL);
-if (!scsidev) {
+false, -1, NULL, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
return -1;
}
dinfo->unit = scsidev->id;
--
1.7.12.4
Re: [Qemu-devel] [PATCH 1/3] scsi-bus: Unify request unref in scsi_req_cancel
Il 18/09/2014 04:36, Fam Zheng ha scritto:
> Before, scsi_req_cancel will take ownership of the canceled request and
> unref it. This is because we didn't know if AIO CB will be called or not
> during the cancelling, so we set the io_canceled flag before calling it,
> and skip to unref in the potentially called callbacks, which is not
> very nice.
>
> Now, bdrv_aio_cancel has a stricter contract that the completion
> callback is always called, so we can remove the special case of
> req->io_canceled.
>
> It will also make implementing asynchronous cancellation easier.
>
> Also, as the existing SCSIReqOps.cancel_io implementations are exactly
> the same, we can move the code to bus for now as we are touching them in
> this patch.
>
> All AIO callbacks in devices, those will be called because of
> bdrv_aio_cancel, should call scsi_req_canceled if req->io_canceled is
> set. That's the uniform place we release the reference we grabbed in
> scsi_req_cancel and notify buses.
>
> Signed-off-by: Fam Zheng
> ---
> hw/scsi/scsi-bus.c | 30 -
> hw/scsi/scsi-disk.c| 59
> ++
> hw/scsi/scsi-generic.c | 28 +---
> include/hw/scsi/scsi.h | 2 +-
> 4 files changed, 37 insertions(+), 82 deletions(-)
>
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 954c607..74172cc 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -1613,7 +1613,6 @@ void scsi_req_continue(SCSIRequest *req)
> {
> if (req->io_canceled) {
> trace_scsi_req_continue_canceled(req->dev->id, req->lun, req->tag);
> -return;
I think this is incorrect; same in scsi_req_data.
As discussed on IRC, scsi-generic is the case where this happens. We
can change it to use the same io_canceled handling as everyone else, so
that the subsequent changes are more mechanical.
> }
> trace_scsi_req_continue(req->dev->id, req->lun, req->tag);
> if (req->cmd.mode == SCSI_XFER_TO_DEV) {
> @@ -1631,7 +1630,6 @@ void scsi_req_data(SCSIRequest *req, int len)
> uint8_t *buf;
> if (req->io_canceled) {
> trace_scsi_req_data_canceled(req->dev->id, req->lun, req->tag, len);
> -return;
> }
> trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
> assert(req->cmd.mode != SCSI_XFER_NONE);
> @@ -1716,35 +1714,37 @@ void scsi_req_complete(SCSIRequest *req, int status)
> scsi_req_unref(req);
> }
>
> +/* Called by the devices when the request is canceled. */
> +void scsi_req_canceled(SCSIRequest *req)
> +{
> +assert(req->io_canceled);
> +if (req->bus->info->cancel) {
> +req->bus->info->cancel(req);
> +}
> +scsi_req_unref(req);
> +}
I think this patch does a bit too much. I would do it like this:
- first, as mentioned above, make scsi-generic follow the usual pattern with
if (req->io_canceled) {
goto done;
}
...
done:
if (!req->io_canceled)
scsi_req_unref(&r->req);
}
- second, remove the scsi_req_unref from the scsi_cancel_io
implementations, and remove the "if" statement around
done:
if (!r->req.io_canceled) {
scsi_req_unref(&r->req);
}
We can do this now that the callback is always invoked, and it
simplifies the reference counting quite a bit.
- third, uninline scsi_cancel_io and introduce scsi_req_canceled
> void scsi_req_cancel(SCSIRequest *req)
> {
> trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
> -if (!req->enqueued) {
> +if (req->io_canceled) {
> return;
> }
> scsi_req_ref(req);
> scsi_req_dequeue(req);
> req->io_canceled = true;
> -if (req->ops->cancel_io) {
> -req->ops->cancel_io(req);
> +if (req->aiocb) {
> +bdrv_aio_cancel(req->aiocb);
> }
> -if (req->bus->info->cancel) {
> -req->bus->info->cancel(req);
> -}
> -scsi_req_unref(req);
> }
>
> void scsi_req_abort(SCSIRequest *req, int status)
> {
> -if (!req->enqueued) {
> +if (req->io_canceled) {
> return;
> }
> scsi_req_ref(req);
> -scsi_req_dequeue(req);
> -req->io_canceled = true;
> -if (req->ops->cancel_io) {
> -req->ops->cancel_io(req);
> -}
> +scsi_req_cancel(req);
This is a problem, because we would complete the request twice: once
from scsi_req_canceled, once in scsi_req_complete below.
Let's just drop scsi_req_abort. The only user can be removed like this:
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 048cfc7..ec5dc0d 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -77,8 +77,9 @@ typedef struct vscsi_req {
SCSIRequest *sreq;
uint32_tqtag; /* qemu tag != srp tag */
boolactive;
-uint32_tdata_len;
boolwriting;
+booldma_error;
+uint32_tdata_len;
uint32_t
[Qemu-devel] [INVITE] OVMF BoF session at the KVM Forum 2014
Hello All, I've been made an offer that I couldn't refuse :) to "organize" a Birds of a Feather session concerning OVMF at the KVM Forum 2014. Interested people, please sign up: http://www.linux-kvm.org/page/KVM_Forum_2014_BOF#OVMF Everyone else: apologies about the noise. Thanks, Laszlo
Re: [Qemu-devel] [PATCH] usb-storage: fix possible memory leak and missing error message
> >> > ---
> >> > hw/usb/dev-storage.c | 11 ++-
> >> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> >> > index 5c51ac0..cabd053 100644
> >> > --- a/hw/usb/dev-storage.c
> >> > +++ b/hw/usb/dev-storage.c
> >> > @@ -599,7 +599,6 @@ static int usb_msd_initfn_storage(USBDevice
> *dev)
> >> > {
> >> > MSDState *s = DO_UPCAST(MSDState, dev, dev);
> >> > BlockDriverState *bs = s->conf.bs;
> >> > -SCSIDevice *scsi_dev;
> >> > Error *err = NULL;
> >> >
> >> > if (!bs) {
> >> > @@ -625,10 +624,12 @@ static int usb_msd_initfn_storage(USBDevice
> *dev)
> >> > usb_desc_init(dev);
> >> > scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
> >> > &usb_msd_scsi_info_storage, NULL);
> >> > -scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs,
> 0, !!s->removable,
> >> > - s->conf.bootindex,
> dev->serial,
> >> > - &err);
> >> > -if (!scsi_dev) {
> >> > +scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> >> > + s->conf.bootindex, dev->serial,
> >> > + &err);
> >> > +if (err) {
> >> > +error_report("%s", error_get_pretty(err));
> >> > +error_free(err);
> >> > return -1;
> >> > }
> >> > s->bus.qbus.allow_hotplug = 0;
> >>
> >> I'd keep the original condition and just fix the error message loss /
> >> error object leak:
> >>
> >> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> >> index ae4efcb..f731b0a 100644
> >> --- a/hw/usb/dev-storage.c
> >> +++ b/hw/usb/dev-storage.c
> >> @@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> >> s->conf.bootindex,
> >> dev->serial,
> >> &err);
> >> if (!scsi_dev) {
> >> +error_report("%s", error_get_pretty(err));
> >> +error_free(err);
> >> return -1;
> >> }
> >> s->bus.qbus.allow_hotplug = 0;
> >>
> >> Partly because it makes the fix more obvious, partly because I prefer
> >> checking the function value when it is available. Matter of taste.
> >>
> > Hmm..
> >
> > The main problem is I'm afraid the scenario when scsi_dev
> > is NULL the err is NULL too in someday. I have fixed a similar issue
> > and you have reviewed. :)
>
> Short answer: doesn't help, and you shouldn't be afraid anyway.
>
> Now the long answer :)
>
> Basic rules about returning errors through Error **errp parameters:
>
> 1. The errp argument may be null.
>
> 2. If it isn't, then it points to null.
>
> 3. The function either succeeds or fails.
>
> 4. If the function succeeds, it doesn't touch *errp.
>
> 5. If the function fails, and
>
>a. if errp isn't null, it creates an Error object and returns it in
>*errp. The caller is responsible for freeing it.
>
>b. if errp is null, no Error object is created.
>
> Corollary: when you pass a non-null errp argument, *errp is null exactly
> on success.
>
> Rule 3 connects the Error contract with what else the function's
> supposed to do on success and on failure. In particular with the return
> value.
>
> Common case: a function returns a pointer to something on success, null
> pointer on failure. When you pass a non-null errp, argument, *errp is
> null exactly when the function value is non-null. In other words,
> either the value or *errp is null.
>
> To detect errors, you can either check the function value, or your errp
> argument.
>
> If you detect an error, you can either handle it locally, or propagate
> it to your caller.
>
> Example: consider a function new_foo() returning a newly allocated Foo
> on success, null on error. Any error we want to propagate.
>
> Checking the errp argument:
>
> Error *local_err = NULL;
>
> foop = new_foo(arg, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> // rely on postcondition !foop (else we'd leak it here)
> return;
> }
>
> Checking the function value:
>
> Error *local_err = NULL;
>
> foop = new_foo(arg, &local_err);
> if (!foop) {
> error_propagate(errp, local_err);
> return;
> }
> // rely on postcondition !local_err (else, we'd leak it here)
>
> No matter which of the two you check, you rely on new_foo()'s
> postcondition "either value or *errp is null".
>
> Therefore, switching from one to the other does not make the code more
> robust.
>
> Checking the function value can be simplified to just
>
> foop = new_foo(arg, errp);
> if (!foop) {
> return;
> }
>
> I use this whenever possible, because it's simpler, and less clutter.
>
> What you fixed is a different, less common case: a function returning a
> list on success, null on failure, where the empty list is represented as
> null. Because a null val
[Qemu-devel] [PATCH v2] pci-hotplug-old: avoid lossing error message
From: Gonglei
When scsi_bus_legacy_add_drive() produce error,
we will loss error message. Using error_report
report it.
Cc: Markus Armbruster
Signed-off-by: Gonglei
---
v2: using original condition instead of local_err (Markus)
---
hw/pci/pci-hotplug-old.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index cf2caeb..d87c469 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -107,6 +107,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
{
SCSIBus *scsibus;
SCSIDevice *scsidev;
+Error *local_err = NULL;
scsibus = (SCSIBus *)
object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
@@ -127,8 +128,10 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
dinfo->bus = scsibus->busnr;
scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
-false, -1, NULL, NULL);
+false, -1, NULL, &local_err);
if (!scsidev) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
return -1;
}
dinfo->unit = scsidev->id;
--
1.7.12.4
Re: [Qemu-devel] [PATCH 2/3] scsi: Introduce scsi_req_cancel_async
Il 18/09/2014 04:36, Fam Zheng ha scritto:
> +QTAILQ_FOREACH_SAFE(ref, &req->cancel_deps, next, next) {
> +SCSIRequest *r = ref->req;
> +assert(r->cancel_dep_count);
> +r->cancel_dep_count--;
> +if (!r->cancel_dep_count && req->bus->info->cancel_dep_complete) {
> +req->bus->info->cancel_dep_complete(r);
> +}
> +QTAILQ_REMOVE(&req->cancel_deps, ref, next);
> +scsi_req_unref(r);
> +g_free(ref);
> +}
I think there is one problem here.
scsi_req_cancel_async can actually be synchronous if you're unlucky
(because bdrv_aio_cancel_async can be synchronous too, for example in
the case of a linux-aio AIOCB). So you could end up calling
cancel_dep_complete even if the caller intends to cancel more requests.
I think it's better to track the count in virtio-scsi instead. You can
initialize it similar to bdrv_aio_multiwrite:
/* Run the aio requests. */
mcb->num_requests = num_reqs;
for (i = 0; i < num_reqs; i++) {
bdrv_co_aio_rw_vector(bs, reqs[i].sector, reqs[i].qiov,
reqs[i].nb_sectors, reqs[i].flags,
multiwrite_cb, mcb,
true);
}
return 0;
and decrement the count on every call to the notifier.
This is independent of the choice to make bdrv_aio_cancel_async
semantics stricter. In the case of bdrv_aio_multiwrite, we know that
bdrv_co_aio_rw_vector is never synchronous, but the code is simply nicer
that way. :)
Paolo
Re: [Qemu-devel] [PATCH 2/3] scsi: Introduce scsi_req_cancel_async
Il 18/09/2014 04:36, Fam Zheng ha scritto: > @@ -552,7 +552,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, > SCSIDevice *d, > SCSIBus *bus = scsi_bus_from_device(d); > BusState *qbus = BUS(bus); > > -req = g_malloc0(reqops->size); > +req = g_malloc0(reqops ? reqops->size : sizeof(SCSIRequest)); Let's just add a NotifierList to SCSIRequest instead, and pass a Notifier to scsi_req_cancel_async. There can be multiple aborts for the same SCSIRequest, we can leave it to the HBA to create a Notifier for each TMF. The notifier can be embedded in a struct to track the request count, as mentioned in the other message I just sent. Paolo
[Qemu-devel] [PATCH 00/19] usb: convert device init to realize
From: Gonglei DeviceClass->init is the old interface, let's convert usb devices to the new realize API. In this way, all the implementations now use error_setg instead of qerror_report/error_report for reporting error. Please review, Thanks! Cc: Markus Armbruster Cc: Paolo Bonzini Cc: Gerd Hoffmann Gonglei (19): usb-storage: fix possible memory leak and missing error message usb-bus: convert USBDeviceClass init to realize usb-net: convert init to realize libusb: convert init to realize libusb: using error_report instead of fprintf usb-hub: convert init to realize dev-storage: convert init to realize dev-storage: usring error_report instead of fprintf/printf dev-uas: convert init to realize dev-uas: using error_report instead of fprintf dev-bluetooth: convert init to realize dev-serial: convert init to realize usb-ccid: convert init to realize dev-hid: convert init to realize dev-wacom: convert init to realize usb-audio: convert init to realize usb-redir: convert init to realize usb-mtp: convert init to realize usb-bus: remove "init" from USBDeviceClass struct hw/usb/bus.c | 79 ++- hw/usb/dev-audio.c| 5 ++- hw/usb/dev-bluetooth.c| 6 ++-- hw/usb/dev-hid.c | 23 ++--- hw/usb/dev-hub.c | 9 +++-- hw/usb/dev-mtp.c | 5 ++- hw/usb/dev-network.c | 9 +++-- hw/usb/dev-serial.c | 15 hw/usb/dev-smartcard-reader.c | 5 ++- hw/usb/dev-storage.c | 42 --- hw/usb/dev-uas.c | 17 +- hw/usb/dev-wacom.c| 5 ++- hw/usb/host-libusb.c | 33 +- hw/usb/redirect.c | 21 +++- include/hw/usb.h | 10 -- 15 files changed, 142 insertions(+), 142 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH 02/19] usb-bus: convert USBDeviceClass init to realize
From: Gonglei
Add "realize/unrealize" in USBDeviceClass, which has errp
as a parameter. So all the implementations now use
error_setg instead of error_report for reporting error.
Note: this patch still keep "init" in USBDeviceClass, and
call kclass->init in usb_device_realize(), avoid breaking
git bisect. After realize all usb devices, will be removed.
Signed-off-by: Gonglei
---
hw/usb/bus.c | 81 +++-
hw/usb/dev-serial.c | 4 +--
hw/usb/dev-storage.c | 11 +--
hw/usb/host-libusb.c | 7 +++--
hw/usb/redirect.c| 6 +++-
include/hw/usb.h | 10 +--
6 files changed, 70 insertions(+), 49 deletions(-)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index c7c4dad..12881cb 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -9,7 +9,7 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev,
int indent);
static char *usb_get_dev_path(DeviceState *dev);
static char *usb_get_fw_dev_path(DeviceState *qdev);
-static int usb_qdev_exit(DeviceState *qdev);
+static void usb_qdev_unrealize(DeviceState *qdev, Error **errp);
static Property usb_props[] = {
DEFINE_PROP_STRING("port", USBDevice, port_path),
@@ -107,13 +107,15 @@ USBBus *usb_bus_find(int busnr)
return NULL;
}
-static int usb_device_init(USBDevice *dev)
+static void usb_device_realize(USBDevice *dev, Error **errp)
{
USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
-if (klass->init) {
-return klass->init(dev);
+
+if (klass->realize) {
+klass->realize(dev, errp);
+} else if (klass->init) {
+klass->init(dev);
}
-return 0;
}
USBDevice *usb_device_find_device(USBDevice *dev, uint8_t addr)
@@ -232,36 +234,41 @@ void usb_device_free_streams(USBDevice *dev, USBEndpoint
**eps, int nr_eps)
}
}
-static int usb_qdev_init(DeviceState *qdev)
+static void usb_qdev_realize(DeviceState *qdev, Error **errp)
{
USBDevice *dev = USB_DEVICE(qdev);
-int rc;
+Error *local_err = NULL;
pstrcpy(dev->product_desc, sizeof(dev->product_desc),
usb_device_get_product_desc(dev));
dev->auto_attach = 1;
QLIST_INIT(&dev->strings);
usb_ep_init(dev);
-rc = usb_claim_port(dev);
-if (rc != 0) {
-return rc;
+
+usb_claim_port(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
}
-rc = usb_device_init(dev);
-if (rc != 0) {
+
+usb_device_realize(dev, &local_err);
+if (local_err) {
usb_release_port(dev);
-return rc;
+error_propagate(errp, local_err);
+return;
}
+
if (dev->auto_attach) {
-rc = usb_device_attach(dev);
-if (rc != 0) {
-usb_qdev_exit(qdev);
-return rc;
+usb_device_attach(dev, &local_err);
+if (local_err) {
+usb_qdev_unrealize(qdev, NULL);
+error_propagate(errp, local_err);
+return;
}
}
-return 0;
}
-static int usb_qdev_exit(DeviceState *qdev)
+static void usb_qdev_unrealize(DeviceState *qdev, Error **errp)
{
USBDevice *dev = USB_DEVICE(qdev);
@@ -272,7 +279,6 @@ static int usb_qdev_exit(DeviceState *qdev)
if (dev->port) {
usb_release_port(dev);
}
-return 0;
}
typedef struct LegacyUSBFactory
@@ -392,7 +398,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
bus->nfree--;
}
-int usb_claim_port(USBDevice *dev)
+void usb_claim_port(USBDevice *dev, Error **errp)
{
USBBus *bus = usb_bus_from_device(dev);
USBPort *port;
@@ -406,9 +412,9 @@ int usb_claim_port(USBDevice *dev)
}
}
if (port == NULL) {
-error_report("Error: usb port %s (bus %s) not found (in use?)",
- dev->port_path, bus->qbus.name);
-return -1;
+error_setg(errp, "Error: usb port %s (bus %s) not found (in use?)",
+ dev->port_path, bus->qbus.name);
+return;
}
} else {
if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)),
"usb-hub") != 0) {
@@ -416,9 +422,9 @@ int usb_claim_port(USBDevice *dev)
usb_create_simple(bus, "usb-hub");
}
if (bus->nfree == 0) {
-error_report("Error: tried to attach usb device %s to a bus "
- "with no free ports", dev->product_desc);
-return -1;
+error_setg(errp, "Error: tried to attach usb device %s to a bus "
+ "with no free ports", dev->product_desc);
+return;
}
port = QTAILQ_FIRST(&bus->free);
}
@@ -432,7 +438,6 @@ int usb_claim_port(USBDevice *dev)
QTAILQ_INSERT_TAIL(&bus->used, port, next);
bus->nused++;
-return 0;
}
void usb_release_port(USBDevice *dev)
@@ -475,7 +480,7 @@ static void usb_mask_to_str(char *dest, size_t size,
}
}
-int usb_device_attach(USBDevice *dev)
+vo
[Qemu-devel] [PATCH 01/19] usb-storage: fix possible memory leak and missing error message
From: Gonglei
When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
be not NULL, which will casue memory leak and missing error message.
Cc: Markus Armbruster
Signed-off-by: Gonglei
---
hw/usb/dev-storage.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae4efcb..f731b0a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
s->conf.bootindex, dev->serial,
&err);
if (!scsi_dev) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
return -1;
}
s->bus.qbus.allow_hotplug = 0;
--
1.7.12.4
[Qemu-devel] [PATCH 19/19] usb-bus: remove "init" from USBDeviceClass struct
From: Gonglei
All usb-bus devices are realized by realize(),
remove init callback function from USBDeviceClass struct.
Signed-off-by: Gonglei
---
hw/usb/bus.c | 2 --
include/hw/usb.h | 2 --
2 files changed, 4 deletions(-)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 12881cb..b375293 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -113,8 +113,6 @@ static void usb_device_realize(USBDevice *dev, Error **errp)
if (klass->realize) {
klass->realize(dev, errp);
-} else if (klass->init) {
-klass->init(dev);
}
}
diff --git a/include/hw/usb.h b/include/hw/usb.h
index 612f09f..8ffbba2 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -273,8 +273,6 @@ typedef void (*USBDeviceUnrealize)(USBDevice *dev, Error
**errp);
typedef struct USBDeviceClass {
DeviceClass parent_class;
-int (*init)(USBDevice *dev);
-
USBDeviceRealize realize;
USBDeviceUnrealize unrealize;
--
1.7.12.4
[Qemu-devel] [PATCH 08/19] dev-storage: usring error_report instead of fprintf/printf
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-storage.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 6dc5f47..681959d 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -409,19 +409,19 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket
*p)
switch (s->mode) {
case USB_MSDM_CBW:
if (p->iov.size != 31) {
-fprintf(stderr, "usb-msd: Bad CBW size");
+error_report("usb-msd: Bad CBW size");
goto fail;
}
usb_packet_copy(p, &cbw, 31);
if (le32_to_cpu(cbw.sig) != 0x43425355) {
-fprintf(stderr, "usb-msd: Bad signature %08x\n",
-le32_to_cpu(cbw.sig));
+error_report("usb-msd: Bad signature %08x",
+ le32_to_cpu(cbw.sig));
goto fail;
}
DPRINTF("Command on LUN %d\n", cbw.lun);
scsi_dev = scsi_device_find(&s->bus, 0, 0, cbw.lun);
if (scsi_dev == NULL) {
-fprintf(stderr, "usb-msd: Bad LUN %d\n", cbw.lun);
+error_report("usb-msd: Bad LUN %d", cbw.lun);
goto fail;
}
tag = le32_to_cpu(cbw.tag);
@@ -680,13 +680,13 @@ static USBDevice *usb_msd_init(USBBus *bus, const char
*filename)
pstrcpy(fmt, len, p2);
qemu_opt_set(opts, "format", fmt);
} else if (*filename != ':') {
-printf("unrecognized USB mass-storage option %s\n", filename);
+error_report("unrecognized USB mass-storage option %s", filename);
return NULL;
}
filename = p1;
}
if (!*filename) {
-printf("block device specification needed\n");
+error_report("block device specification needed");
return NULL;
}
qemu_opt_set(opts, "file", filename);
--
1.7.12.4
[Qemu-devel] [PATCH 06/19] usb-hub: convert init to realize
From: Gonglei
In this way, all the implementations now use
error_setg instead of error_report for reporting error.
Signed-off-by: Gonglei
---
hw/usb/dev-hub.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 7492174..8e3f7a8 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -511,15 +511,15 @@ static USBPortOps usb_hub_port_ops = {
.complete = usb_hub_complete,
};
-static int usb_hub_initfn(USBDevice *dev)
+static void usb_hub_realize(USBDevice *dev, Error **errp)
{
USBHubState *s = DO_UPCAST(USBHubState, dev, dev);
USBHubPort *port;
int i;
if (dev->port->hubcount == 5) {
-error_report("usb hub chain too deep");
-return -1;
+error_setg(errp, "usb hub chain too deep");
+return;
}
usb_desc_create_serial(dev);
@@ -533,7 +533,6 @@ static int usb_hub_initfn(USBDevice *dev)
usb_port_location(&port->port, dev->port, i+1);
}
usb_hub_handle_reset(dev);
-return 0;
}
static const VMStateDescription vmstate_usb_hub_port = {
@@ -564,7 +563,7 @@ static void usb_hub_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_hub_initfn;
+uc->realize = usb_hub_realize;
uc->product_desc = "QEMU USB Hub";
uc->usb_desc = &desc_hub;
uc->find_device= usb_hub_find_device;
--
1.7.12.4
[Qemu-devel] [PATCH 14/19] dev-hid: convert init to realize
From: Gonglei
In this way, all the implementations now use
error_setg instead of error_report for reporting error.
Signed-off-by: Gonglei
---
hw/usb/dev-hid.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 67a57f1..7e9d2d6 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -582,7 +582,7 @@ static int usb_hid_initfn(USBDevice *dev, int kind)
return 0;
}
-static int usb_tablet_initfn(USBDevice *dev)
+static void usb_tablet_realize(USBDevice *dev, Error **errp)
{
USBHIDState *us = DO_UPCAST(USBHIDState, dev, dev);
@@ -594,22 +594,21 @@ static int usb_tablet_initfn(USBDevice *dev)
dev->usb_desc = &desc_tablet2;
break;
default:
-error_report("Invalid usb version %d for usb-tabler (must be 1 or 2)",
- us->usb_version);
-return -1;
+error_setg(errp, "Invalid usb version %d for usb-tablet "
+ "(must be 1 or 2)", us->usb_version);
}
-return usb_hid_initfn(dev, HID_TABLET);
+usb_hid_initfn(dev, HID_TABLET);
}
-static int usb_mouse_initfn(USBDevice *dev)
+static void usb_mouse_realize(USBDevice *dev, Error **errp)
{
-return usb_hid_initfn(dev, HID_MOUSE);
+usb_hid_initfn(dev, HID_MOUSE);
}
-static int usb_keyboard_initfn(USBDevice *dev)
+static void usb_keyboard_realize(USBDevice *dev, Error **errp)
{
-return usb_hid_initfn(dev, HID_KEYBOARD);
+usb_hid_initfn(dev, HID_KEYBOARD);
}
static int usb_ptr_post_load(void *opaque, int version_id)
@@ -669,7 +668,7 @@ static void usb_tablet_class_initfn(ObjectClass *klass,
void *data)
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
usb_hid_class_initfn(klass, data);
-uc->init = usb_tablet_initfn;
+uc->realize = usb_tablet_realize;
uc->product_desc = "QEMU USB Tablet";
dc->vmsd = &vmstate_usb_ptr;
dc->props = usb_tablet_properties;
@@ -689,7 +688,7 @@ static void usb_mouse_class_initfn(ObjectClass *klass, void
*data)
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
usb_hid_class_initfn(klass, data);
-uc->init = usb_mouse_initfn;
+uc->realize = usb_mouse_realize;
uc->product_desc = "QEMU USB Mouse";
uc->usb_desc = &desc_mouse;
dc->vmsd = &vmstate_usb_ptr;
@@ -714,7 +713,7 @@ static void usb_keyboard_class_initfn(ObjectClass *klass,
void *data)
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
usb_hid_class_initfn(klass, data);
-uc->init = usb_keyboard_initfn;
+uc->realize = usb_keyboard_realize;
uc->product_desc = "QEMU USB Keyboard";
uc->usb_desc = &desc_keyboard;
dc->vmsd = &vmstate_usb_kbd;
--
1.7.12.4
[Qemu-devel] [PATCH 04/19] libusb: convert init to realize
From: Gonglei
In this way, all the implementations now use
error_setg instead of error_report for reporting error.
Signed-off-by: Gonglei
---
hw/usb/host-libusb.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 9f92705..863be64 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -951,21 +951,21 @@ static void usb_host_exit_notifier(struct Notifier *n,
void *data)
}
}
-static int usb_host_initfn(USBDevice *udev)
+static void usb_host_realize(USBDevice *udev, Error **errp)
{
USBHostDevice *s = USB_HOST_DEVICE(udev);
if (s->match.vendor_id > 0x) {
-error_report("vendorid out of range");
-return -1;
+error_setg(errp, "vendorid out of range");
+return;
}
if (s->match.product_id > 0x) {
-error_report("productid out of range");
-return -1;
+error_setg(errp, "productid out of range");
+return;
}
if (s->match.addr > 127) {
-error_report("hostaddr out of range");
-return -1;
+error_setg(errp, "hostaddr out of range");
+return;
}
loglevel = s->loglevel;
@@ -980,7 +980,6 @@ static int usb_host_initfn(USBDevice *udev)
QTAILQ_INSERT_TAIL(&hostdevs, s, next);
add_boot_device_path(s->bootindex, &udev->qdev, NULL);
usb_host_auto_check(NULL);
-return 0;
}
static void usb_host_handle_destroy(USBDevice *udev)
@@ -1480,7 +1479,7 @@ static void usb_host_class_initfn(ObjectClass *klass,
void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_host_initfn;
+uc->realize = usb_host_realize;
uc->product_desc = "USB Host Device";
uc->cancel_packet = usb_host_cancel_packet;
uc->handle_data= usb_host_handle_data;
--
1.7.12.4
[Qemu-devel] [PATCH 16/19] usb-audio: convert init to realize
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-audio.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 7b9957b..83ebe83 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -628,7 +628,7 @@ static void usb_audio_handle_destroy(USBDevice *dev)
streambuf_fini(&s->out.buf);
}
-static int usb_audio_initfn(USBDevice *dev)
+static void usb_audio_realize(USBDevice *dev, Error **errp)
{
USBAudioState *s = DO_UPCAST(USBAudioState, dev, dev);
@@ -651,7 +651,6 @@ static int usb_audio_initfn(USBDevice *dev)
s, output_callback, &s->out.as);
AUD_set_volume_out(s->out.voice, s->out.mute, s->out.vol[0],
s->out.vol[1]);
AUD_set_active_out(s->out.voice, 0);
-return 0;
}
static const VMStateDescription vmstate_usb_audio = {
@@ -676,7 +675,7 @@ static void usb_audio_class_init(ObjectClass *klass, void
*data)
set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
k->product_desc = "QEMU USB Audio Interface";
k->usb_desc = &desc_audio;
-k->init = usb_audio_initfn;
+k->realize = usb_audio_realize;
k->handle_reset = usb_audio_handle_reset;
k->handle_control = usb_audio_handle_control;
k->handle_data= usb_audio_handle_data;
--
1.7.12.4
[Qemu-devel] [PATCH 10/19] dev-uas: using error_report instead of fprintf
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-uas.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 884c003..1508064 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -13,6 +13,7 @@
#include "qemu/option.h"
#include "qemu/config-file.h"
#include "trace.h"
+#include "qemu/error-report.h"
#include "hw/usb.h"
#include "hw/usb/desc.h"
@@ -648,7 +649,7 @@ static void usb_uas_handle_control(USBDevice *dev,
USBPacket *p,
if (ret >= 0) {
return;
}
-fprintf(stderr, "%s: unhandled control request\n", __func__);
+error_report("%s: unhandled control request", __func__);
p->status = USB_RET_STALL;
}
@@ -814,8 +815,8 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
*p)
usb_uas_task(uas, &iu);
break;
default:
-fprintf(stderr, "%s: unknown command iu: id 0x%x\n",
-__func__, iu.hdr.id);
+error_report("%s: unknown command iu: id 0x%x",
+ __func__, iu.hdr.id);
p->status = USB_RET_STALL;
break;
}
@@ -861,7 +862,7 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
*p)
p->status = USB_RET_ASYNC;
break;
} else {
-fprintf(stderr, "%s: no inflight request\n", __func__);
+error_report("%s: no inflight request", __func__);
p->status = USB_RET_STALL;
break;
}
@@ -879,7 +880,7 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
*p)
usb_uas_start_next_transfer(uas);
break;
default:
-fprintf(stderr, "%s: invalid endpoint %d\n", __func__, p->ep->nr);
+error_report("%s: invalid endpoint %d", __func__, p->ep->nr);
p->status = USB_RET_STALL;
break;
}
--
1.7.12.4
[Qemu-devel] [PATCH 18/19] usb-mtp: convert init to realize
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-mtp.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 0820046..108ece8 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1060,7 +1060,7 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket
*p)
}
}
-static int usb_mtp_initfn(USBDevice *dev)
+static void usb_mtp_realize(USBDevice *dev, Error **errp)
{
MTPState *s = DO_UPCAST(MTPState, dev, dev);
@@ -1075,7 +1075,6 @@ static int usb_mtp_initfn(USBDevice *dev)
s->desc = g_strdup("none");
}
}
-return 0;
}
static const VMStateDescription vmstate_usb_mtp = {
@@ -1100,7 +1099,7 @@ static void usb_mtp_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_mtp_initfn;
+uc->realize= usb_mtp_realize;
uc->product_desc = "QEMU USB MTP";
uc->usb_desc = &desc;
uc->cancel_packet = usb_mtp_cancel_packet;
--
1.7.12.4
[Qemu-devel] [PATCH 12/19] dev-serial: convert init to realize
From: Gonglei
In this way, all the implementations now use
error_setg instead of error_report for reporting error.
Signed-off-by: Gonglei
---
hw/usb/dev-serial.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 178ecb2..3384db6 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -471,7 +471,7 @@ static void usb_serial_event(void *opaque, int event)
}
}
-static int usb_serial_initfn(USBDevice *dev)
+static void usb_serial_realize(USBDevice *dev, Error **errp)
{
USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
@@ -480,8 +480,8 @@ static int usb_serial_initfn(USBDevice *dev)
dev->auto_attach = 0;
if (!s->cs) {
-error_report("Property chardev is required");
-return -1;
+error_setg(errp, "Property chardev is required");
+return;
}
qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read,
@@ -491,7 +491,6 @@ static int usb_serial_initfn(USBDevice *dev)
if (s->cs->be_open && !dev->attached) {
usb_device_attach(dev, NULL);
}
-return 0;
}
static USBDevice *usb_serial_init(USBBus *bus, const char *filename)
@@ -582,7 +581,7 @@ static void usb_serial_class_initfn(ObjectClass *klass,
void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_serial_initfn;
+uc->realize = usb_serial_realize;
uc->product_desc = "QEMU USB Serial";
uc->usb_desc = &desc_serial;
uc->handle_reset = usb_serial_handle_reset;
@@ -610,7 +609,7 @@ static void usb_braille_class_initfn(ObjectClass *klass,
void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_serial_initfn;
+uc->realize = usb_serial_realize;
uc->product_desc = "QEMU USB Braille";
uc->usb_desc = &desc_braille;
uc->handle_reset = usb_serial_handle_reset;
--
1.7.12.4
[Qemu-devel] [PATCH 15/19] dev-wacom: convert init to realize
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-wacom.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
index 1b73fd0..217d48e 100644
--- a/hw/usb/dev-wacom.c
+++ b/hw/usb/dev-wacom.c
@@ -335,14 +335,13 @@ static void usb_wacom_handle_destroy(USBDevice *dev)
}
}
-static int usb_wacom_initfn(USBDevice *dev)
+static void usb_wacom_realize(USBDevice *dev, Error **errp)
{
USBWacomState *s = DO_UPCAST(USBWacomState, dev, dev);
usb_desc_create_serial(dev);
usb_desc_init(dev);
s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
s->changed = 1;
-return 0;
}
static const VMStateDescription vmstate_usb_wacom = {
@@ -357,7 +356,7 @@ static void usb_wacom_class_init(ObjectClass *klass, void
*data)
uc->product_desc = "QEMU PenPartner Tablet";
uc->usb_desc = &desc_wacom;
-uc->init = usb_wacom_initfn;
+uc->realize = usb_wacom_realize;
uc->handle_reset = usb_wacom_handle_reset;
uc->handle_control = usb_wacom_handle_control;
uc->handle_data= usb_wacom_handle_data;
--
1.7.12.4
[Qemu-devel] [PATCH 13/19] usb-ccid: convert init to realize
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-smartcard-reader.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 470e69f..442f487 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1304,7 +1304,7 @@ static int ccid_card_init(DeviceState *qdev)
return ret;
}
-static int ccid_initfn(USBDevice *dev)
+static void ccid_realize(USBDevice *dev, Error **errp)
{
USBCCIDState *s = DO_UPCAST(USBCCIDState, dev, dev);
@@ -1332,7 +1332,6 @@ static int ccid_initfn(USBDevice *dev)
ccid_reset_parameters(s);
ccid_reset(s);
s->debug = parse_debug_env("QEMU_CCID_DEBUG", D_VERBOSE, s->debug);
-return 0;
}
static int ccid_post_load(void *opaque, int version_id)
@@ -1441,7 +1440,7 @@ static void ccid_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = ccid_initfn;
+uc->realize = ccid_realize;
uc->product_desc = "QEMU USB CCID";
uc->usb_desc = &desc_ccid;
uc->handle_reset = ccid_handle_reset;
--
1.7.12.4
[Qemu-devel] [PATCH 09/19] dev-uas: convert init to realize
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-uas.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 9832385..884c003 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -892,7 +892,7 @@ static void usb_uas_handle_destroy(USBDevice *dev)
qemu_bh_delete(uas->status_bh);
}
-static int usb_uas_init(USBDevice *dev)
+static void usb_uas_realize(USBDevice *dev, Error **errp)
{
UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
@@ -905,8 +905,6 @@ static int usb_uas_init(USBDevice *dev)
scsi_bus_new(&uas->bus, sizeof(uas->bus), DEVICE(dev),
&usb_uas_scsi_info, NULL);
-
-return 0;
}
static const VMStateDescription vmstate_usb_uas = {
@@ -928,7 +926,7 @@ static void usb_uas_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_uas_init;
+uc->realize = usb_uas_realize;
uc->product_desc = desc_strings[STR_PRODUCT];
uc->usb_desc = &desc;
uc->cancel_packet = usb_uas_cancel_io;
--
1.7.12.4
[Qemu-devel] [PATCH 17/19] usb-redir: convert init to realize
From: Gonglei
In this way, all the implementations now use
error_setg instead of qerror_report for reporting error.
Signed-off-by: Gonglei
---
hw/usb/redirect.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 95158b3..e2c9896 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1361,14 +1361,14 @@ static void usbredir_init_endpoints(USBRedirDevice *dev)
}
}
-static int usbredir_initfn(USBDevice *udev)
+static void usbredir_realize(USBDevice *udev, Error **errp)
{
USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
int i;
if (dev->cs == NULL) {
-qerror_report(QERR_MISSING_PARAMETER, "chardev");
-return -1;
+error_set(errp, QERR_MISSING_PARAMETER, "chardev");
+return;
}
if (dev->filter_str) {
@@ -1376,9 +1376,9 @@ static int usbredir_initfn(USBDevice *udev)
&dev->filter_rules,
&dev->filter_rules_count);
if (i) {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, "filter",
- "a usb device filter string");
-return -1;
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, "filter",
+ "a usb device filter string");
+return;
}
}
@@ -1402,7 +1402,6 @@ static int usbredir_initfn(USBDevice *udev)
qemu_add_vm_change_state_handler(usbredir_vm_state_change, dev);
add_boot_device_path(dev->bootindex, &udev->qdev, NULL);
-return 0;
}
static void usbredir_cleanup_device_queues(USBRedirDevice *dev)
@@ -2481,7 +2480,7 @@ static void usbredir_class_initfn(ObjectClass *klass,
void *data)
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
-uc->init = usbredir_initfn;
+uc->realize= usbredir_realize;
uc->product_desc = "USB Redirection Device";
uc->handle_destroy = usbredir_handle_destroy;
uc->cancel_packet = usbredir_cancel_packet;
--
1.7.12.4
[Qemu-devel] [PATCH 05/19] libusb: using error_report instead of fprintf
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/host-libusb.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 863be64..0650910 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -275,7 +275,7 @@ static void usb_host_libusb_error(const char *func, int rc)
} else {
errname = "?";
}
-fprintf(stderr, "%s: %d [%s]\n", func, rc, errname);
+error_report("%s: %d [%s]", func, rc, errname);
}
/* */
@@ -1376,14 +1376,13 @@ static int usb_host_alloc_streams(USBDevice *udev,
USBEndpoint **eps,
if (rc < 0) {
usb_host_libusb_error("libusb_alloc_streams", rc);
} else if (rc != streams) {
-fprintf(stderr,
-"libusb_alloc_streams: got less streams then requested %d < %d\n",
-rc, streams);
+error_report("libusb_alloc_streams: got less streams "
+ "then requested %d < %d", rc, streams);
}
return (rc == streams) ? 0 : -1;
#else
-fprintf(stderr, "libusb_alloc_streams: error not implemented\n");
+error_report("libusb_alloc_streams: error not implemented");
return -1;
#endif
}
--
1.7.12.4
[Qemu-devel] [PATCH 03/19] usb-net: convert init to realize
From: Gonglei
meanwhile, qerror_report_err() is a transitional interface to
help with converting existing HMP commands to QMP. It should
not be used elsewhere.
Signed-off-by: Gonglei
---
hw/usb/dev-network.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 518d536..686bd69 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -27,7 +27,7 @@
#include "hw/usb.h"
#include "hw/usb/desc.h"
#include "net/net.h"
-#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
#include "qemu/queue.h"
#include "qemu/config-file.h"
#include "sysemu/sysemu.h"
@@ -1341,7 +1341,7 @@ static NetClientInfo net_usbnet_info = {
.cleanup = usbnet_cleanup,
};
-static int usb_net_initfn(USBDevice *dev)
+static void usb_net_realize(USBDevice *dev, Error **errrp)
{
USBNetState *s = DO_UPCAST(USBNetState, dev, dev);
@@ -1373,7 +1373,6 @@ static int usb_net_initfn(USBDevice *dev)
usb_desc_set_string(dev, STRING_ETHADDR, s->usbstring_mac);
add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0");
-return 0;
}
static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
@@ -1392,7 +1391,7 @@ static USBDevice *usb_net_init(USBBus *bus, const char
*cmdline)
idx = net_client_init(opts, 0, &local_err);
if (local_err) {
-qerror_report_err(local_err);
+error_report("%s", error_get_pretty(local_err));
error_free(local_err);
return NULL;
}
@@ -1421,7 +1420,7 @@ static void usb_net_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_net_initfn;
+uc->realize = usb_net_realize;
uc->product_desc = "QEMU USB Network Interface";
uc->usb_desc = &desc_net;
uc->handle_reset = usb_net_handle_reset;
--
1.7.12.4
[Qemu-devel] [PATCH 07/19] dev-storage: convert init to realize
From: Gonglei
In this way, all the implementations now use
error_setg instead of error_report for reporting error.
Signed-off-by: Gonglei
---
hw/usb/dev-storage.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index a9f31f0..6dc5f47 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -595,7 +595,7 @@ static const struct SCSIBusInfo usb_msd_scsi_info_bot = {
.load_request = usb_msd_load_request,
};
-static int usb_msd_initfn_storage(USBDevice *dev)
+static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
{
MSDState *s = DO_UPCAST(MSDState, dev, dev);
BlockDriverState *bs = s->conf.bs;
@@ -603,8 +603,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
Error *err = NULL;
if (!bs) {
-error_report("drive property not set");
-return -1;
+error_setg(errp, "drive property not set");
+return;
}
blkconf_serial(&s->conf, &dev->serial);
@@ -629,9 +629,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
s->conf.bootindex, dev->serial,
&err);
if (!scsi_dev) {
-error_report("%s", error_get_pretty(err));
-error_free(err);
-return -1;
+error_propagate(errp, err);
+return;
}
s->bus.qbus.allow_hotplug = 0;
usb_msd_handle_reset(dev);
@@ -644,11 +643,9 @@ static int usb_msd_initfn_storage(USBDevice *dev)
autostart = 0;
}
}
-
-return 0;
}
-static int usb_msd_initfn_bot(USBDevice *dev)
+static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
{
MSDState *s = DO_UPCAST(MSDState, dev, dev);
@@ -658,8 +655,6 @@ static int usb_msd_initfn_bot(USBDevice *dev)
&usb_msd_scsi_info_bot, NULL);
s->bus.qbus.allow_hotplug = 0;
usb_msd_handle_reset(dev);
-
-return 0;
}
static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
@@ -765,7 +760,7 @@ static void usb_msd_class_initfn_storage(ObjectClass
*klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_msd_initfn_storage;
+uc->realize = usb_msd_realize_storage;
dc->props = msd_properties;
usb_msd_class_initfn_common(klass);
}
@@ -774,7 +769,7 @@ static void usb_msd_class_initfn_bot(ObjectClass *klass,
void *data)
{
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_msd_initfn_bot;
+uc->realize = usb_msd_realize_bot;
usb_msd_class_initfn_common(klass);
}
--
1.7.12.4
[Qemu-devel] [PATCH 11/19] dev-bluetooth: convert init to realize
From: Gonglei
Signed-off-by: Gonglei
---
hw/usb/dev-bluetooth.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
index a76e581..7db9921 100644
--- a/hw/usb/dev-bluetooth.c
+++ b/hw/usb/dev-bluetooth.c
@@ -501,7 +501,7 @@ static void usb_bt_handle_destroy(USBDevice *dev)
s->hci->acl_recv = NULL;
}
-static int usb_bt_initfn(USBDevice *dev)
+static void usb_bt_realize(USBDevice *dev, Error **errp)
{
struct USBBtState *s = DO_UPCAST(struct USBBtState, dev, dev);
@@ -516,8 +516,6 @@ static int usb_bt_initfn(USBDevice *dev)
s->hci->acl_recv = usb_bt_out_hci_packet_acl;
usb_bt_handle_reset(&s->dev);
s->intr = usb_ep_get(dev, USB_TOKEN_IN, USB_EVT_EP);
-
-return 0;
}
static USBDevice *usb_bt_init(USBBus *bus, const char *cmdline)
@@ -560,7 +558,7 @@ static void usb_bt_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_bt_initfn;
+uc->realize = usb_bt_realize;
uc->product_desc = "QEMU BT dongle";
uc->usb_desc = &desc_bluetooth;
uc->handle_reset = usb_bt_handle_reset;
--
1.7.12.4
Re: [Qemu-devel] migration: qemu-coroutine-lock.c:141: qemu_co_mutex_unlock: Assertion `mutex->locked == 1' failed
Il 18/09/2014 05:26, Alexey Kardashevskiy ha scritto: > On 09/18/2014 01:07 AM, Stefan Hajnoczi wrote: >> On Wed, Sep 17, 2014 at 2:44 PM, Alexey Kardashevskiy wrote: >>> On 09/17/2014 07:25 PM, Paolo Bonzini wrote: >>> btw any better idea of a hack to try? Testers are pushing me - they want to >>> upgrade the broken setup and I am blocking them :) Thanks! >> >> Paolo's qemu_co_mutex_lock(&s->lock) idea in qcow2_invalidate_cache() >> is good. Have you tried that patch? > > > Yes, did not help. > >> >> I haven't checked the qcow2 code whether that works properly across >> bdrv_close() (is the lock freed?) but in principle that's how you >> protect against concurrent I/O. > > I thought we have to avoid qemu_coroutine_yield() in this particular case. > I fail to see how the locks may help if we still do yeild. But the whole > thing is already way behind of my understanding :) For example - how many > BlockDriverState things are layered here? NBD -> QCOW2 -> RAW? No, this is an NBD server. So we have three users of the same QCOW2 image: migration, NBD server and virtio disk (not active while the bug happens, and thus not depicted): NBD server ->QCOW2 <- migration | v File The problem is that the NBD server accesses the QCOW2 image while migration does qcow2_invalidate_cache. Paolo
[Qemu-devel] [PATCH] block: Catch simultaneous usage of options and their aliases
While thinking about precedence of conflicting block device options from
different sources, I noticed that you can specify both an option and its
legacy alias at the same time (e.g. readonly=on,read-only=off). Rather
than specifying the order of precedence, we should simply forbid such
combinations.
Signed-off-by: Kevin Wolf
---
blockdev.c | 46 +++---
tests/qemu-iotests/051 | 23 +++
tests/qemu-iotests/051.out | 45 +
3 files changed, 99 insertions(+), 15 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a194d04..7a9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -532,12 +532,22 @@ err_no_opts:
return NULL;
}
-static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to)
+static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to,
+Error **errp)
{
const char *value;
+if (*errp) {
+return;
+}
+
value = qemu_opt_get(opts, from);
if (value) {
+if (qemu_opt_find(opts, to)) {
+error_setg(errp, "'%s' and its alias '%s' can't be used at the "
+ "same time", to, from);
+return;
+}
qemu_opt_set(opts, to, value);
qemu_opt_unset(opts, from);
}
@@ -643,26 +653,32 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
Error *local_err = NULL;
/* Change legacy command line options into QMP ones */
-qemu_opt_rename(all_opts, "iops", "throttling.iops-total");
-qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read");
-qemu_opt_rename(all_opts, "iops_wr", "throttling.iops-write");
+qemu_opt_rename(all_opts, "iops", "throttling.iops-total", &local_err);
+qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read", &local_err);
+qemu_opt_rename(all_opts, "iops_wr", "throttling.iops-write", &local_err);
-qemu_opt_rename(all_opts, "bps", "throttling.bps-total");
-qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read");
-qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write");
+qemu_opt_rename(all_opts, "bps", "throttling.bps-total", &local_err);
+qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read", &local_err);
+qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write", &local_err);
-qemu_opt_rename(all_opts, "iops_max", "throttling.iops-total-max");
-qemu_opt_rename(all_opts, "iops_rd_max", "throttling.iops-read-max");
-qemu_opt_rename(all_opts, "iops_wr_max", "throttling.iops-write-max");
+qemu_opt_rename(all_opts, "iops_max", "throttling.iops-total-max",
&local_err);
+qemu_opt_rename(all_opts, "iops_rd_max", "throttling.iops-read-max",
&local_err);
+qemu_opt_rename(all_opts, "iops_wr_max", "throttling.iops-write-max",
&local_err);
-qemu_opt_rename(all_opts, "bps_max", "throttling.bps-total-max");
-qemu_opt_rename(all_opts, "bps_rd_max", "throttling.bps-read-max");
-qemu_opt_rename(all_opts, "bps_wr_max", "throttling.bps-write-max");
+qemu_opt_rename(all_opts, "bps_max", "throttling.bps-total-max",
&local_err);
+qemu_opt_rename(all_opts, "bps_rd_max", "throttling.bps-read-max",
&local_err);
+qemu_opt_rename(all_opts, "bps_wr_max", "throttling.bps-write-max",
&local_err);
qemu_opt_rename(all_opts,
-"iops_size", "throttling.iops-size");
+"iops_size", "throttling.iops-size", &local_err);
+
+qemu_opt_rename(all_opts, "readonly", "read-only", &local_err);
-qemu_opt_rename(all_opts, "readonly", "read-only");
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
+return NULL;
+}
value = qemu_opt_get(all_opts, "cache");
if (value) {
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index a41334e..11c858f 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -199,6 +199,29 @@ run_qemu -drive file.driver=raw
run_qemu -drive foo=bar
echo
+echo === Specifying both an option and its legacy alias ===
+echo
+
+run_qemu -drive file="$TEST_IMG",iops=1234,throttling.iops-total=5678
+run_qemu -drive file="$TEST_IMG",iops_rd=1234,throttling.iops-read=5678
+run_qemu -drive file="$TEST_IMG",iops_wr=1234,throttling.iops-write=5678
+
+run_qemu -drive file="$TEST_IMG",bps=1234,throttling.bps-total=5678
+run_qemu -drive file="$TEST_IMG",bps_rd=1234,throttling.bps-read=5678
+run_qemu -drive file="$TEST_IMG",bps_wr=1234,throttling.bps-write=5678
+
+run_qemu -drive file="$TEST_IMG",iops_max=1234,throttling.iops-total-max=5678
+run_qemu -drive file="$TEST_IMG",iops_rd_max=1234,throttling.iops-read-max=5678
+run_qemu -drive
file="$TEST_IMG",iops_wr_max=1234,throttling.iops-write-max=5678
+
+run_qemu -drive file="$TEST_IMG",bps_max=1234,throttling.bps-total-max=5678
+run_qemu -drive file="$TEST_IMG",bps_rd_m
Re: [Qemu-devel] [PATCH 02/19] usb-bus: convert USBDeviceClass init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> @@ -460,7 +460,7 @@ static void usb_serial_event(void *opaque, int event)
> break;
> case CHR_EVENT_OPENED:
> if (!s->dev.attached) {
> -usb_device_attach(&s->dev);
> +usb_device_attach(&s->dev, NULL);
After this patch, this stops printing the error if the attach fails.
Please add qerror_report_err.
> }
> break;
> case CHR_EVENT_CLOSED:
> @@ -489,7 +489,7 @@ static int usb_serial_initfn(USBDevice *dev)
> usb_serial_handle_reset(dev);
>
> if (s->cs->be_open && !dev->attached) {
> -usb_device_attach(dev);
> +usb_device_attach(dev, NULL);
This too. Please add qerror_report_err here too and, in this case,
please make patch 12 pass the errp instead of using qerror_report_err.
Paolo
> }
> return 0;
Re: [Qemu-devel] [PATCH 02/19] usb-bus: convert USBDeviceClass init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> static void usb_msd_password_cb(void *opaque, int err)
> {
> MSDState *s = opaque;
> +Error *local_err = NULL;
>
> -if (!err)
> -err = usb_device_attach(&s->dev);
> +if (!err) {
> +usb_device_attach(&s->dev, &local_err);
> +}
>
> -if (err)
> +if (local_err) {
> +error_report("%s", error_get_pretty(local_err));
I think this should use qerror_report_err.
Paolo
Re: [Qemu-devel] [PATCH V3 0/7] cpu/acpi: convert cpu hot plug to hotplug_handler API
Hi Igor, The issues you mentioned in previous comments are fixed in this version. Could please help to review it? Thanks, Gu On 09/17/2014 09:23 AM, Gu Zheng wrote: > Previously we use cpu_added_notifiers to register cpu hotplug notifier > callback > which is not able to pass/handle errors, so we switch it to unified hotplug > handler API which allows to pass errors and would allow to cancel device_add > in case of error. > Thanks very much for Igor's review and suggestion. > > v3: > -deal with start-up cpus in pc_cpu_plug as Igor suggested. > > v2: > -Add 3 new patches(5/7,6/7,7/7), delete original patch 5/5. > 1/5-->1/7 > 2/5-->2/7 > 3/5-->3/7 > 4/5-->4/7 > Patch 1/7: > -add errp argument to catch error. > -return error instead of aborting if cpu id is invalid. > -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add. > Patch 3/7: > -remove unused AcpiCpuHotplug_add directly. > Patch 5/7: > -switch the last user of cpu hotplug notifier to hotplug handler API, and > remove the unused cpu hotplug notify. > Patch 6/7: > -split the function rename (just cleanup) into single patch. > Patch 7/7: > -introduce help function acpi_set_local_sts to keep the bit setting in > one place. > > Gu Zheng (7): > acpi/cpu: add cpu hotplug callback function to match hotplug_handler > API > acpi:ich9: convert cpu hotplug handle to hotplug_handler API > acpi:piix4: convert cpu hotplug handle to hotplug_handler API > pc: add cpu hotplug handler to PC_MACHINE > pc: Update rtc_cmos in pc_cpu_plug > cpu-hotplug: rename function for better readability > acpi/cpu-hotplug: introduce help function to keep bit setting in one > place > > hw/acpi/cpu_hotplug.c | 35 > hw/acpi/ich9.c| 18 -- > hw/acpi/piix4.c | 18 +++--- > hw/i386/pc.c | 51 > + > include/hw/acpi/cpu_hotplug.h |7 +++-- > include/hw/acpi/ich9.h|1 - > include/sysemu/sysemu.h |3 -- > qom/cpu.c | 10 > 8 files changed, 69 insertions(+), 74 deletions(-) >
Re: [Qemu-devel] [PATCH 04/19] libusb: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> In this way, all the implementations now use
> error_setg instead of error_report for reporting error.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/host-libusb.c | 17 -
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index 9f92705..863be64 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -951,21 +951,21 @@ static void usb_host_exit_notifier(struct Notifier *n,
> void *data)
> }
> }
>
> -static int usb_host_initfn(USBDevice *udev)
> +static void usb_host_realize(USBDevice *udev, Error **errp)
> {
> USBHostDevice *s = USB_HOST_DEVICE(udev);
>
> if (s->match.vendor_id > 0x) {
> -error_report("vendorid out of range");
> -return -1;
> +error_setg(errp, "vendorid out of range");
> +return;
> }
> if (s->match.product_id > 0x) {
> -error_report("productid out of range");
> -return -1;
> +error_setg(errp, "productid out of range");
> +return;
> }
> if (s->match.addr > 127) {
> -error_report("hostaddr out of range");
> -return -1;
> +error_setg(errp, "hostaddr out of range");
> +return;
> }
>
> loglevel = s->loglevel;
> @@ -980,7 +980,6 @@ static int usb_host_initfn(USBDevice *udev)
> QTAILQ_INSERT_TAIL(&hostdevs, s, next);
> add_boot_device_path(s->bootindex, &udev->qdev, NULL);
> usb_host_auto_check(NULL);
> -return 0;
> }
>
> static void usb_host_handle_destroy(USBDevice *udev)
> @@ -1480,7 +1479,7 @@ static void usb_host_class_initfn(ObjectClass *klass,
> void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_host_initfn;
> +uc->realize = usb_host_realize;
> uc->product_desc = "USB Host Device";
> uc->cancel_packet = usb_host_cancel_packet;
> uc->handle_data= usb_host_handle_data;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 06/19] usb-hub: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> In this way, all the implementations now use
> error_setg instead of error_report for reporting error.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-hub.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
> index 7492174..8e3f7a8 100644
> --- a/hw/usb/dev-hub.c
> +++ b/hw/usb/dev-hub.c
> @@ -511,15 +511,15 @@ static USBPortOps usb_hub_port_ops = {
> .complete = usb_hub_complete,
> };
>
> -static int usb_hub_initfn(USBDevice *dev)
> +static void usb_hub_realize(USBDevice *dev, Error **errp)
> {
> USBHubState *s = DO_UPCAST(USBHubState, dev, dev);
> USBHubPort *port;
> int i;
>
> if (dev->port->hubcount == 5) {
> -error_report("usb hub chain too deep");
> -return -1;
> +error_setg(errp, "usb hub chain too deep");
> +return;
> }
>
> usb_desc_create_serial(dev);
> @@ -533,7 +533,6 @@ static int usb_hub_initfn(USBDevice *dev)
> usb_port_location(&port->port, dev->port, i+1);
> }
> usb_hub_handle_reset(dev);
> -return 0;
> }
>
> static const VMStateDescription vmstate_usb_hub_port = {
> @@ -564,7 +563,7 @@ static void usb_hub_class_initfn(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_hub_initfn;
> +uc->realize = usb_hub_realize;
> uc->product_desc = "QEMU USB Hub";
> uc->usb_desc = &desc_hub;
> uc->find_device= usb_hub_find_device;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 05/19] libusb: using error_report instead of fprintf
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/host-libusb.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index 863be64..0650910 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -275,7 +275,7 @@ static void usb_host_libusb_error(const char *func, int
> rc)
> } else {
> errname = "?";
> }
> -fprintf(stderr, "%s: %d [%s]\n", func, rc, errname);
> +error_report("%s: %d [%s]", func, rc, errname);
> }
>
> /*
> */
> @@ -1376,14 +1376,13 @@ static int usb_host_alloc_streams(USBDevice *udev,
> USBEndpoint **eps,
> if (rc < 0) {
> usb_host_libusb_error("libusb_alloc_streams", rc);
> } else if (rc != streams) {
> -fprintf(stderr,
> -"libusb_alloc_streams: got less streams then requested %d <
> %d\n",
> -rc, streams);
> +error_report("libusb_alloc_streams: got less streams "
> + "then requested %d < %d", rc, streams);
> }
>
> return (rc == streams) ? 0 : -1;
> #else
> -fprintf(stderr, "libusb_alloc_streams: error not implemented\n");
> +error_report("libusb_alloc_streams: error not implemented");
> return -1;
> #endif
> }
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 09/19] dev-uas: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-uas.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
> index 9832385..884c003 100644
> --- a/hw/usb/dev-uas.c
> +++ b/hw/usb/dev-uas.c
> @@ -892,7 +892,7 @@ static void usb_uas_handle_destroy(USBDevice *dev)
> qemu_bh_delete(uas->status_bh);
> }
>
> -static int usb_uas_init(USBDevice *dev)
> +static void usb_uas_realize(USBDevice *dev, Error **errp)
> {
> UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
>
> @@ -905,8 +905,6 @@ static int usb_uas_init(USBDevice *dev)
>
> scsi_bus_new(&uas->bus, sizeof(uas->bus), DEVICE(dev),
> &usb_uas_scsi_info, NULL);
> -
> -return 0;
> }
>
> static const VMStateDescription vmstate_usb_uas = {
> @@ -928,7 +926,7 @@ static void usb_uas_class_initfn(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_uas_init;
> +uc->realize = usb_uas_realize;
> uc->product_desc = desc_strings[STR_PRODUCT];
> uc->usb_desc = &desc;
> uc->cancel_packet = usb_uas_cancel_io;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 08/19] dev-storage: usring error_report instead of fprintf/printf
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-storage.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 6dc5f47..681959d 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -409,19 +409,19 @@ static void usb_msd_handle_data(USBDevice *dev,
> USBPacket *p)
> switch (s->mode) {
> case USB_MSDM_CBW:
> if (p->iov.size != 31) {
> -fprintf(stderr, "usb-msd: Bad CBW size");
> +error_report("usb-msd: Bad CBW size");
> goto fail;
> }
> usb_packet_copy(p, &cbw, 31);
> if (le32_to_cpu(cbw.sig) != 0x43425355) {
> -fprintf(stderr, "usb-msd: Bad signature %08x\n",
> -le32_to_cpu(cbw.sig));
> +error_report("usb-msd: Bad signature %08x",
> + le32_to_cpu(cbw.sig));
> goto fail;
> }
> DPRINTF("Command on LUN %d\n", cbw.lun);
> scsi_dev = scsi_device_find(&s->bus, 0, 0, cbw.lun);
> if (scsi_dev == NULL) {
> -fprintf(stderr, "usb-msd: Bad LUN %d\n", cbw.lun);
> +error_report("usb-msd: Bad LUN %d", cbw.lun);
> goto fail;
> }
> tag = le32_to_cpu(cbw.tag);
> @@ -680,13 +680,13 @@ static USBDevice *usb_msd_init(USBBus *bus, const char
> *filename)
> pstrcpy(fmt, len, p2);
> qemu_opt_set(opts, "format", fmt);
> } else if (*filename != ':') {
> -printf("unrecognized USB mass-storage option %s\n", filename);
> +error_report("unrecognized USB mass-storage option %s",
> filename);
> return NULL;
> }
> filename = p1;
> }
> if (!*filename) {
> -printf("block device specification needed\n");
> +error_report("block device specification needed");
> return NULL;
> }
> qemu_opt_set(opts, "file", filename);
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 12/19] dev-serial: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> In this way, all the implementations now use
> error_setg instead of error_report for reporting error.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-serial.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 178ecb2..3384db6 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -471,7 +471,7 @@ static void usb_serial_event(void *opaque, int event)
> }
> }
>
> -static int usb_serial_initfn(USBDevice *dev)
> +static void usb_serial_realize(USBDevice *dev, Error **errp)
> {
> USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
>
> @@ -480,8 +480,8 @@ static int usb_serial_initfn(USBDevice *dev)
> dev->auto_attach = 0;
>
> if (!s->cs) {
> -error_report("Property chardev is required");
> -return -1;
> +error_setg(errp, "Property chardev is required");
> +return;
> }
>
> qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read,
> @@ -491,7 +491,6 @@ static int usb_serial_initfn(USBDevice *dev)
> if (s->cs->be_open && !dev->attached) {
> usb_device_attach(dev, NULL);
> }
> -return 0;
> }
>
> static USBDevice *usb_serial_init(USBBus *bus, const char *filename)
> @@ -582,7 +581,7 @@ static void usb_serial_class_initfn(ObjectClass *klass,
> void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_serial_initfn;
> +uc->realize = usb_serial_realize;
> uc->product_desc = "QEMU USB Serial";
> uc->usb_desc = &desc_serial;
> uc->handle_reset = usb_serial_handle_reset;
> @@ -610,7 +609,7 @@ static void usb_braille_class_initfn(ObjectClass *klass,
> void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_serial_initfn;
> +uc->realize = usb_serial_realize;
> uc->product_desc = "QEMU USB Braille";
> uc->usb_desc = &desc_braille;
> uc->handle_reset = usb_serial_handle_reset;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 07/19] dev-storage: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> In this way, all the implementations now use
> error_setg instead of error_report for reporting error.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-storage.c | 21 -
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index a9f31f0..6dc5f47 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -595,7 +595,7 @@ static const struct SCSIBusInfo usb_msd_scsi_info_bot = {
> .load_request = usb_msd_load_request,
> };
>
> -static int usb_msd_initfn_storage(USBDevice *dev)
> +static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
> {
> MSDState *s = DO_UPCAST(MSDState, dev, dev);
> BlockDriverState *bs = s->conf.bs;
> @@ -603,8 +603,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> Error *err = NULL;
>
> if (!bs) {
> -error_report("drive property not set");
> -return -1;
> +error_setg(errp, "drive property not set");
> +return;
> }
>
> blkconf_serial(&s->conf, &dev->serial);
> @@ -629,9 +629,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> s->conf.bootindex, dev->serial,
> &err);
> if (!scsi_dev) {
> -error_report("%s", error_get_pretty(err));
> -error_free(err);
> -return -1;
> +error_propagate(errp, err);
> +return;
> }
> s->bus.qbus.allow_hotplug = 0;
> usb_msd_handle_reset(dev);
> @@ -644,11 +643,9 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> autostart = 0;
> }
> }
> -
> -return 0;
> }
>
> -static int usb_msd_initfn_bot(USBDevice *dev)
> +static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
> {
> MSDState *s = DO_UPCAST(MSDState, dev, dev);
>
> @@ -658,8 +655,6 @@ static int usb_msd_initfn_bot(USBDevice *dev)
> &usb_msd_scsi_info_bot, NULL);
> s->bus.qbus.allow_hotplug = 0;
> usb_msd_handle_reset(dev);
> -
> -return 0;
> }
>
> static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
> @@ -765,7 +760,7 @@ static void usb_msd_class_initfn_storage(ObjectClass
> *klass, void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_msd_initfn_storage;
> +uc->realize = usb_msd_realize_storage;
> dc->props = msd_properties;
> usb_msd_class_initfn_common(klass);
> }
> @@ -774,7 +769,7 @@ static void usb_msd_class_initfn_bot(ObjectClass *klass,
> void *data)
> {
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_msd_initfn_bot;
> +uc->realize = usb_msd_realize_bot;
> usb_msd_class_initfn_common(klass);
> }
>
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 13/19] usb-ccid: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-smartcard-reader.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index 470e69f..442f487 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -1304,7 +1304,7 @@ static int ccid_card_init(DeviceState *qdev)
> return ret;
> }
>
> -static int ccid_initfn(USBDevice *dev)
> +static void ccid_realize(USBDevice *dev, Error **errp)
> {
> USBCCIDState *s = DO_UPCAST(USBCCIDState, dev, dev);
>
> @@ -1332,7 +1332,6 @@ static int ccid_initfn(USBDevice *dev)
> ccid_reset_parameters(s);
> ccid_reset(s);
> s->debug = parse_debug_env("QEMU_CCID_DEBUG", D_VERBOSE, s->debug);
> -return 0;
> }
>
> static int ccid_post_load(void *opaque, int version_id)
> @@ -1441,7 +1440,7 @@ static void ccid_class_initfn(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = ccid_initfn;
> +uc->realize = ccid_realize;
> uc->product_desc = "QEMU USB CCID";
> uc->usb_desc = &desc_ccid;
> uc->handle_reset = ccid_handle_reset;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 15/19] dev-wacom: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-wacom.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
> index 1b73fd0..217d48e 100644
> --- a/hw/usb/dev-wacom.c
> +++ b/hw/usb/dev-wacom.c
> @@ -335,14 +335,13 @@ static void usb_wacom_handle_destroy(USBDevice *dev)
> }
> }
>
> -static int usb_wacom_initfn(USBDevice *dev)
> +static void usb_wacom_realize(USBDevice *dev, Error **errp)
> {
> USBWacomState *s = DO_UPCAST(USBWacomState, dev, dev);
> usb_desc_create_serial(dev);
> usb_desc_init(dev);
> s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
> s->changed = 1;
> -return 0;
> }
>
> static const VMStateDescription vmstate_usb_wacom = {
> @@ -357,7 +356,7 @@ static void usb_wacom_class_init(ObjectClass *klass, void
> *data)
>
> uc->product_desc = "QEMU PenPartner Tablet";
> uc->usb_desc = &desc_wacom;
> -uc->init = usb_wacom_initfn;
> +uc->realize = usb_wacom_realize;
> uc->handle_reset = usb_wacom_handle_reset;
> uc->handle_control = usb_wacom_handle_control;
> uc->handle_data= usb_wacom_handle_data;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 14/19] dev-hid: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> In this way, all the implementations now use
> error_setg instead of error_report for reporting error.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-hid.c | 23 +++
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
> index 67a57f1..7e9d2d6 100644
> --- a/hw/usb/dev-hid.c
> +++ b/hw/usb/dev-hid.c
> @@ -582,7 +582,7 @@ static int usb_hid_initfn(USBDevice *dev, int kind)
> return 0;
Please change usb_hid_initfn to return void.
> }
>
> -static int usb_tablet_initfn(USBDevice *dev)
> +static void usb_tablet_realize(USBDevice *dev, Error **errp)
> {
> USBHIDState *us = DO_UPCAST(USBHIDState, dev, dev);
>
> @@ -594,22 +594,21 @@ static int usb_tablet_initfn(USBDevice *dev)
> dev->usb_desc = &desc_tablet2;
> break;
> default:
> -error_report("Invalid usb version %d for usb-tabler (must be 1 or
> 2)",
> - us->usb_version);
> -return -1;
> +error_setg(errp, "Invalid usb version %d for usb-tablet "
> + "(must be 1 or 2)", us->usb_version);
Missing return here.
Paolo
> }
>
> -return usb_hid_initfn(dev, HID_TABLET);
> +usb_hid_initfn(dev, HID_TABLET);
> }
>
> -static int usb_mouse_initfn(USBDevice *dev)
> +static void usb_mouse_realize(USBDevice *dev, Error **errp)
> {
> -return usb_hid_initfn(dev, HID_MOUSE);
> +usb_hid_initfn(dev, HID_MOUSE);
> }
>
> -static int usb_keyboard_initfn(USBDevice *dev)
> +static void usb_keyboard_realize(USBDevice *dev, Error **errp)
> {
> -return usb_hid_initfn(dev, HID_KEYBOARD);
> +usb_hid_initfn(dev, HID_KEYBOARD);
> }
>
> static int usb_ptr_post_load(void *opaque, int version_id)
> @@ -669,7 +668,7 @@ static void usb_tablet_class_initfn(ObjectClass *klass,
> void *data)
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> usb_hid_class_initfn(klass, data);
> -uc->init = usb_tablet_initfn;
> +uc->realize = usb_tablet_realize;
> uc->product_desc = "QEMU USB Tablet";
> dc->vmsd = &vmstate_usb_ptr;
> dc->props = usb_tablet_properties;
> @@ -689,7 +688,7 @@ static void usb_mouse_class_initfn(ObjectClass *klass,
> void *data)
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> usb_hid_class_initfn(klass, data);
> -uc->init = usb_mouse_initfn;
> +uc->realize = usb_mouse_realize;
> uc->product_desc = "QEMU USB Mouse";
> uc->usb_desc = &desc_mouse;
> dc->vmsd = &vmstate_usb_ptr;
> @@ -714,7 +713,7 @@ static void usb_keyboard_class_initfn(ObjectClass *klass,
> void *data)
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> usb_hid_class_initfn(klass, data);
> -uc->init = usb_keyboard_initfn;
> +uc->realize = usb_keyboard_realize;
> uc->product_desc = "QEMU USB Keyboard";
> uc->usb_desc = &desc_keyboard;
> dc->vmsd = &vmstate_usb_kbd;
>
Re: [Qemu-devel] [PATCH 10/19] dev-uas: using error_report instead of fprintf
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-uas.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
> index 884c003..1508064 100644
> --- a/hw/usb/dev-uas.c
> +++ b/hw/usb/dev-uas.c
> @@ -13,6 +13,7 @@
> #include "qemu/option.h"
> #include "qemu/config-file.h"
> #include "trace.h"
> +#include "qemu/error-report.h"
>
> #include "hw/usb.h"
> #include "hw/usb/desc.h"
> @@ -648,7 +649,7 @@ static void usb_uas_handle_control(USBDevice *dev,
> USBPacket *p,
> if (ret >= 0) {
> return;
> }
> -fprintf(stderr, "%s: unhandled control request\n", __func__);
> +error_report("%s: unhandled control request", __func__);
> p->status = USB_RET_STALL;
> }
>
> @@ -814,8 +815,8 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
> *p)
> usb_uas_task(uas, &iu);
> break;
> default:
> -fprintf(stderr, "%s: unknown command iu: id 0x%x\n",
> -__func__, iu.hdr.id);
> +error_report("%s: unknown command iu: id 0x%x",
> + __func__, iu.hdr.id);
> p->status = USB_RET_STALL;
> break;
> }
> @@ -861,7 +862,7 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
> *p)
> p->status = USB_RET_ASYNC;
> break;
> } else {
> -fprintf(stderr, "%s: no inflight request\n", __func__);
> +error_report("%s: no inflight request", __func__);
> p->status = USB_RET_STALL;
> break;
> }
> @@ -879,7 +880,7 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
> *p)
> usb_uas_start_next_transfer(uas);
> break;
> default:
> -fprintf(stderr, "%s: invalid endpoint %d\n", __func__, p->ep->nr);
> +error_report("%s: invalid endpoint %d", __func__, p->ep->nr);
> p->status = USB_RET_STALL;
> break;
> }
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 03/19] usb-net: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> meanwhile, qerror_report_err() is a transitional interface to
> help with converting existing HMP commands to QMP. It should
> not be used elsewhere.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-network.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
> index 518d536..686bd69 100644
> --- a/hw/usb/dev-network.c
> +++ b/hw/usb/dev-network.c
> @@ -27,7 +27,7 @@
> #include "hw/usb.h"
> #include "hw/usb/desc.h"
> #include "net/net.h"
> -#include "qapi/qmp/qerror.h"
> +#include "qemu/error-report.h"
> #include "qemu/queue.h"
> #include "qemu/config-file.h"
> #include "sysemu/sysemu.h"
> @@ -1341,7 +1341,7 @@ static NetClientInfo net_usbnet_info = {
> .cleanup = usbnet_cleanup,
> };
>
> -static int usb_net_initfn(USBDevice *dev)
> +static void usb_net_realize(USBDevice *dev, Error **errrp)
> {
> USBNetState *s = DO_UPCAST(USBNetState, dev, dev);
>
> @@ -1373,7 +1373,6 @@ static int usb_net_initfn(USBDevice *dev)
> usb_desc_set_string(dev, STRING_ETHADDR, s->usbstring_mac);
>
> add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0");
> -return 0;
> }
>
> static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
> @@ -1392,7 +1391,7 @@ static USBDevice *usb_net_init(USBBus *bus, const char
> *cmdline)
>
> idx = net_client_init(opts, 0, &local_err);
> if (local_err) {
> -qerror_report_err(local_err);
> +error_report("%s", error_get_pretty(local_err));
> error_free(local_err);
> return NULL;
> }
> @@ -1421,7 +1420,7 @@ static void usb_net_class_initfn(ObjectClass *klass,
> void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_net_initfn;
> +uc->realize = usb_net_realize;
> uc->product_desc = "QEMU USB Network Interface";
> uc->usb_desc = &desc_net;
> uc->handle_reset = usb_net_handle_reset;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 16/19] usb-audio: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-audio.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
> index 7b9957b..83ebe83 100644
> --- a/hw/usb/dev-audio.c
> +++ b/hw/usb/dev-audio.c
> @@ -628,7 +628,7 @@ static void usb_audio_handle_destroy(USBDevice *dev)
> streambuf_fini(&s->out.buf);
> }
>
> -static int usb_audio_initfn(USBDevice *dev)
> +static void usb_audio_realize(USBDevice *dev, Error **errp)
> {
> USBAudioState *s = DO_UPCAST(USBAudioState, dev, dev);
>
> @@ -651,7 +651,6 @@ static int usb_audio_initfn(USBDevice *dev)
> s, output_callback, &s->out.as);
> AUD_set_volume_out(s->out.voice, s->out.mute, s->out.vol[0],
> s->out.vol[1]);
> AUD_set_active_out(s->out.voice, 0);
> -return 0;
> }
>
> static const VMStateDescription vmstate_usb_audio = {
> @@ -676,7 +675,7 @@ static void usb_audio_class_init(ObjectClass *klass, void
> *data)
> set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
> k->product_desc = "QEMU USB Audio Interface";
> k->usb_desc = &desc_audio;
> -k->init = usb_audio_initfn;
> +k->realize = usb_audio_realize;
> k->handle_reset = usb_audio_handle_reset;
> k->handle_control = usb_audio_handle_control;
> k->handle_data= usb_audio_handle_data;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 11/19] dev-bluetooth: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-bluetooth.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
> index a76e581..7db9921 100644
> --- a/hw/usb/dev-bluetooth.c
> +++ b/hw/usb/dev-bluetooth.c
> @@ -501,7 +501,7 @@ static void usb_bt_handle_destroy(USBDevice *dev)
> s->hci->acl_recv = NULL;
> }
>
> -static int usb_bt_initfn(USBDevice *dev)
> +static void usb_bt_realize(USBDevice *dev, Error **errp)
> {
> struct USBBtState *s = DO_UPCAST(struct USBBtState, dev, dev);
>
> @@ -516,8 +516,6 @@ static int usb_bt_initfn(USBDevice *dev)
> s->hci->acl_recv = usb_bt_out_hci_packet_acl;
> usb_bt_handle_reset(&s->dev);
> s->intr = usb_ep_get(dev, USB_TOKEN_IN, USB_EVT_EP);
> -
> -return 0;
> }
>
> static USBDevice *usb_bt_init(USBBus *bus, const char *cmdline)
> @@ -560,7 +558,7 @@ static void usb_bt_class_initfn(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_bt_initfn;
> +uc->realize = usb_bt_realize;
> uc->product_desc = "QEMU BT dongle";
> uc->usb_desc = &desc_bluetooth;
> uc->handle_reset = usb_bt_handle_reset;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 17/19] usb-redir: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> In this way, all the implementations now use
> error_setg instead of qerror_report for reporting error.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/redirect.c | 15 +++
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 95158b3..e2c9896 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -1361,14 +1361,14 @@ static void usbredir_init_endpoints(USBRedirDevice
> *dev)
> }
> }
>
> -static int usbredir_initfn(USBDevice *udev)
> +static void usbredir_realize(USBDevice *udev, Error **errp)
> {
> USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
> int i;
>
> if (dev->cs == NULL) {
> -qerror_report(QERR_MISSING_PARAMETER, "chardev");
> -return -1;
> +error_set(errp, QERR_MISSING_PARAMETER, "chardev");
> +return;
> }
>
> if (dev->filter_str) {
> @@ -1376,9 +1376,9 @@ static int usbredir_initfn(USBDevice *udev)
> &dev->filter_rules,
> &dev->filter_rules_count);
> if (i) {
> -qerror_report(QERR_INVALID_PARAMETER_VALUE, "filter",
> - "a usb device filter string");
> -return -1;
> +error_set(errp, QERR_INVALID_PARAMETER_VALUE, "filter",
> + "a usb device filter string");
> +return;
> }
> }
>
> @@ -1402,7 +1402,6 @@ static int usbredir_initfn(USBDevice *udev)
>
> qemu_add_vm_change_state_handler(usbredir_vm_state_change, dev);
> add_boot_device_path(dev->bootindex, &udev->qdev, NULL);
> -return 0;
> }
>
> static void usbredir_cleanup_device_queues(USBRedirDevice *dev)
> @@ -2481,7 +2480,7 @@ static void usbredir_class_initfn(ObjectClass *klass,
> void *data)
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> -uc->init = usbredir_initfn;
> +uc->realize= usbredir_realize;
> uc->product_desc = "USB Redirection Device";
> uc->handle_destroy = usbredir_handle_destroy;
> uc->cancel_packet = usbredir_cancel_packet;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 18/19] usb-mtp: convert init to realize
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-mtp.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 0820046..108ece8 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1060,7 +1060,7 @@ static void usb_mtp_handle_data(USBDevice *dev,
> USBPacket *p)
> }
> }
>
> -static int usb_mtp_initfn(USBDevice *dev)
> +static void usb_mtp_realize(USBDevice *dev, Error **errp)
> {
> MTPState *s = DO_UPCAST(MTPState, dev, dev);
>
> @@ -1075,7 +1075,6 @@ static int usb_mtp_initfn(USBDevice *dev)
> s->desc = g_strdup("none");
> }
> }
> -return 0;
> }
>
> static const VMStateDescription vmstate_usb_mtp = {
> @@ -1100,7 +1099,7 @@ static void usb_mtp_class_initfn(ObjectClass *klass,
> void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
>
> -uc->init = usb_mtp_initfn;
> +uc->realize= usb_mtp_realize;
> uc->product_desc = "QEMU USB MTP";
> uc->usb_desc = &desc;
> uc->cancel_packet = usb_mtp_cancel_packet;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 01/19] usb-storage: fix possible memory leak and missing error message
Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
> be not NULL, which will casue memory leak and missing error message.
>
> Cc: Markus Armbruster
> Signed-off-by: Gonglei
> ---
> hw/usb/dev-storage.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index ae4efcb..f731b0a 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> s->conf.bootindex, dev->serial,
> &err);
> if (!scsi_dev) {
> +error_report("%s", error_get_pretty(err));
> +error_free(err);
> return -1;
> }
> s->bus.qbus.allow_hotplug = 0;
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 19/19] usb-bus: remove "init" from USBDeviceClass struct
Il 18/09/2014 11:33, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> All usb-bus devices are realized by realize(),
> remove init callback function from USBDeviceClass struct.
>
> Signed-off-by: Gonglei
> ---
> hw/usb/bus.c | 2 --
> include/hw/usb.h | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index 12881cb..b375293 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -113,8 +113,6 @@ static void usb_device_realize(USBDevice *dev, Error
> **errp)
>
> if (klass->realize) {
> klass->realize(dev, errp);
> -} else if (klass->init) {
> -klass->init(dev);
> }
> }
>
> diff --git a/include/hw/usb.h b/include/hw/usb.h
> index 612f09f..8ffbba2 100644
> --- a/include/hw/usb.h
> +++ b/include/hw/usb.h
> @@ -273,8 +273,6 @@ typedef void (*USBDeviceUnrealize)(USBDevice *dev, Error
> **errp);
> typedef struct USBDeviceClass {
> DeviceClass parent_class;
>
> -int (*init)(USBDevice *dev);
> -
> USBDeviceRealize realize;
> USBDeviceUnrealize unrealize;
>
>
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 02/19] usb-bus: convert USBDeviceClass init to realize
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Thursday, September 18, 2014 6:08 PM
> Subject: Re: [PATCH 02/19] usb-bus: convert USBDeviceClass init to realize
>
> Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> > @@ -460,7 +460,7 @@ static void usb_serial_event(void *opaque, int event)
> > break;
> > case CHR_EVENT_OPENED:
> > if (!s->dev.attached) {
> > -usb_device_attach(&s->dev);
> > +usb_device_attach(&s->dev, NULL);
>
> After this patch, this stops printing the error if the attach fails.
> Please add qerror_report_err.
>
Good catch. Will fix it in v2.
Thanks a lot for your fast reviewing! :)
> > }
> > break;
> > case CHR_EVENT_CLOSED:
> > @@ -489,7 +489,7 @@ static int usb_serial_initfn(USBDevice *dev)
> > usb_serial_handle_reset(dev);
> >
> > if (s->cs->be_open && !dev->attached) {
> > -usb_device_attach(dev);
> > +usb_device_attach(dev, NULL);
>
> This too. Please add qerror_report_err here too and, in this case,
> please make patch 12 pass the errp instead of using qerror_report_err.
>
OK. Got it.
Best regards,
-Gonglei
> Paolo
>
> > }
> > return 0;
Re: [Qemu-devel] [PATCH 02/19] usb-bus: convert USBDeviceClass init to realize
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Thursday, September 18, 2014 6:09 PM
> Subject: Re: [PATCH 02/19] usb-bus: convert USBDeviceClass init to realize
>
> Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> > static void usb_msd_password_cb(void *opaque, int err)
> > {
> > MSDState *s = opaque;
> > +Error *local_err = NULL;
> >
> > -if (!err)
> > -err = usb_device_attach(&s->dev);
> > +if (!err) {
> > +usb_device_attach(&s->dev, &local_err);
> > +}
> >
> > -if (err)
> > +if (local_err) {
> > +error_report("%s", error_get_pretty(local_err));
>
> I think this should use qerror_report_err.
>
OK.
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH 14/19] dev-hid: convert init to realize
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Thursday, September 18, 2014 6:16 PM
> Subject: Re: [PATCH 14/19] dev-hid: convert init to realize
>
> Il 18/09/2014 11:32, arei.gong...@huawei.com ha scritto:
> > From: Gonglei
> >
> > In this way, all the implementations now use
> > error_setg instead of error_report for reporting error.
> >
> > Signed-off-by: Gonglei
> > ---
> > hw/usb/dev-hid.c | 23 +++
> > 1 file changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
> > index 67a57f1..7e9d2d6 100644
> > --- a/hw/usb/dev-hid.c
> > +++ b/hw/usb/dev-hid.c
> > @@ -582,7 +582,7 @@ static int usb_hid_initfn(USBDevice *dev, int kind)
> > return 0;
>
> Please change usb_hid_initfn to return void.
>
OK.
> > }
> >
> > -static int usb_tablet_initfn(USBDevice *dev)
> > +static void usb_tablet_realize(USBDevice *dev, Error **errp)
> > {
> > USBHIDState *us = DO_UPCAST(USBHIDState, dev, dev);
> >
> > @@ -594,22 +594,21 @@ static int usb_tablet_initfn(USBDevice *dev)
> > dev->usb_desc = &desc_tablet2;
> > break;
> > default:
> > -error_report("Invalid usb version %d for usb-tabler (must be 1 or
> 2)",
> > - us->usb_version);
> > -return -1;
> > +error_setg(errp, "Invalid usb version %d for usb-tablet "
> > + "(must be 1 or 2)", us->usb_version);
>
> Missing return here.
>
Good catch! Will fix it in v2. Thanks a lot!
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH] block/archipelago: Fix typo in qemu_archipelago_truncate()
On Tue, Sep 16, 2014 at 12:17:10PM +0300, Chrysostomos Nanakos wrote: > > Chrysostomos Nanakos (1): > block/archipelago: Fix typo in qemu_archipelago_truncate() > > block/archipelago.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > -- > 1.7.10.4 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgp_BUb4dN9yW.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v4] async: aio_context_new(): Handle event_notifier_init failure
On Wed, Sep 17, 2014 at 04:48:34PM +0300, Chrysostomos Nanakos wrote:
> diff --git a/iothread.c b/iothread.c
> index d9403cf..6e394a0 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -17,6 +17,7 @@
> #include "block/aio.h"
> #include "sysemu/iothread.h"
> #include "qmp-commands.h"
> +#include "qemu/error-report.h"
>
> #define IOTHREADS_PATH "/objects"
>
> @@ -53,6 +54,9 @@ static void iothread_instance_finalize(Object *obj)
> {
> IOThread *iothread = IOTHREAD(obj);
>
> +if (!iothread->ctx) {
> +return;
> +}
> iothread->stopping = true;
> aio_notify(iothread->ctx);
> qemu_thread_join(&iothread->thread);
> @@ -63,10 +67,15 @@ static void iothread_instance_finalize(Object *obj)
>
> static void iothread_complete(UserCreatable *obj, Error **errp)
> {
> +Error *local_error = NULL;
> IOThread *iothread = IOTHREAD(obj);
>
> iothread->stopping = false;
> -iothread->ctx = aio_context_new();
> +iothread->ctx = aio_context_new(&local_error);
> +if (!iothread->ctx) {
> +error_propagate(errp, local_error);
> +return;
> +}
> iothread->thread_id = -1;
Please move this under ->stopping = false so that ->thread_id is
initialized. That way qmp_query_iothreads() will display thread_id -1
for failed IOThread objects instead of an uninitialized value.
Normally QEMU should exit or the IOThread object should be deleted
before anyone has a chance to call qmp_query_iothreads() but you never
know so let's get the lifecycle right...
> diff --git a/main-loop.c b/main-loop.c
> index 3cc79f8..2f83d80 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -126,10 +126,11 @@ void qemu_notify_event(void)
>
> static GArray *gpollfds;
>
> -int qemu_init_main_loop(void)
> +int qemu_init_main_loop(Error **errp)
> {
> int ret;
> GSource *src;
> +Error *local_error = NULL;
>
> init_clocks();
>
> @@ -138,8 +139,12 @@ int qemu_init_main_loop(void)
> return ret;
> }
>
> +qemu_aio_context = aio_context_new(&local_error);
> +if (!qemu_aio_context) {
> +error_propagate(errp, local_error);
> +return -1;
This function returns -errno so -1 would be -EPERM. Please use an
actual errno constant like -EMFILE.
> diff --git a/tests/test-aio.c b/tests/test-aio.c
> index c6a8713..029716f 100644
> --- a/tests/test-aio.c
> +++ b/tests/test-aio.c
> @@ -14,6 +14,7 @@
> #include "block/aio.h"
> #include "qemu/timer.h"
> #include "qemu/sockets.h"
> +#include "qemu/error-report.h"
>
> static AioContext *ctx;
>
> @@ -810,11 +811,18 @@ static void test_source_timer_schedule(void)
>
> int main(int argc, char **argv)
> {
> +Error *local_error;
local_error = NULL
It must be NULL otherwise error_setg() will read uninitialized memory
(it checks that the error hasn't been set yet because Error objects can
only be set once).
> diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
> index f40b7fc..57c0836 100644
> --- a/tests/test-thread-pool.c
> +++ b/tests/test-thread-pool.c
> @@ -4,6 +4,7 @@
> #include "block/thread-pool.h"
> #include "block/block.h"
> #include "qemu/timer.h"
> +#include "qemu/error-report.h"
>
> static AioContext *ctx;
> static ThreadPool *pool;
> @@ -205,10 +206,17 @@ static void test_cancel(void)
> int main(int argc, char **argv)
> {
> int ret;
> +Error *local_error;
Same.
> diff --git a/tests/test-throttle.c b/tests/test-throttle.c
> index 000ae31..42141a0 100644
> --- a/tests/test-throttle.c
> +++ b/tests/test-throttle.c
> @@ -14,6 +14,7 @@
> #include
> #include "block/aio.h"
> #include "qemu/throttle.h"
> +#include "qemu/error-report.h"
>
> static AioContext *ctx;
> static LeakyBucketbkt;
> @@ -492,10 +493,17 @@ static void test_accounting(void)
> int main(int argc, char **argv)
> {
> GSource *src;
> +Error *local_error;
Same.
pgpc6jRRUAVRb.pgp
Description: PGP signature
Re: [Qemu-devel] qemu process stuck in Rl state
On Wed, Sep 17, 2014 at 11:56:57PM +0400, Andrey Korolyov wrote: > I`ve faced an issue with qemu VMs with very large uptime spans - half > of year or so. They are hanging in running state forever and are not > killable in any imaginable fashion. Tried to freeze it via freezer cg > without any luck. VM itself went unresponsive with zero cpu > consumption after reaching 'forever running' point. > > I am going to reset the host in a couple of hours, so any timed ideas > for debugging this state will be very appreciated. A couple of shots at figuring out what the process is doing: cat /proc/$PID/stack cat /proc/$PID/syscall gdb $PID (gdb) thread apply all bt pgpymqAHvcv1H.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 1/6] block/qapi: Add cache information to query-block
Kevin Wolf writes:
> Signed-off-by: Kevin Wolf
> ---
> block/qapi.c | 10 ++
> hmp.c | 8
> qapi/block-core.json | 4 +++-
> tests/qemu-iotests/051.out | 1 +
> tests/qemu-iotests/067.out | 10 +-
> 5 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 9733ebd..6b43bc3 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -46,6 +46,16 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState
> *bs)
> info->encrypted = bs->encrypted;
> info->encryption_key_missing = bdrv_key_required(bs);
>
> +info->cache = g_new(BlockdevCacheOptions, 1);
> +*info->cache = (BlockdevCacheOptions) {
> +.writeback = bdrv_enable_write_cache(bs),
> +.has_writeback = true,
> +.direct = !!(bs->open_flags & BDRV_O_NOCACHE),
> +.has_direct = true,
> +.no_flush = !!(bs->open_flags & BDRV_O_NO_FLUSH),
> +.has_no_flush = true,
> +};
> +
Awkward optionality. I'm going to discuss this below, in the context of
the schema change.
> if (bs->node_name[0]) {
> info->has_node_name = true;
> info->node_name = g_strdup(bs->node_name);
> diff --git a/hmp.c b/hmp.c
> index 40a90da..c3846b8 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -294,6 +294,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
> {
> BlockInfoList *block_list, *info;
> ImageInfo *image_info;
> +BlockDeviceInfo *inserted;
> const char *device = qdict_get_try_str(qdict, "device");
> bool verbose = qdict_get_try_bool(qdict, "verbose", 0);
>
> @@ -335,6 +336,13 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
> continue;
> }
>
> +inserted = info->value->inserted;
> +
> +monitor_printf(mon, "Cache mode: %s%s%s\n",
> + inserted->cache->writeback ? "writeback" :
> "writethrough",
> + inserted->cache->direct ? ", direct" : "",
> + inserted->cache->no_flush ? ", ignore flushes" : "");
> +
If !inserted->cache->writeback && inserted->cache->direct, this prints
"Cache mode: , writeback".
> if (info->value->inserted->has_backing_file) {
> monitor_printf(mon,
> "Backing file: %s "
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 95dcd81..c53b587 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -238,6 +238,8 @@
> #
> # @iops_size: #optional an I/O size in bytes (Since 1.7)
> #
> +# @cache: the cache mode used for the block device (since: 2.2)
> +#
> # Since: 0.14.0
> #
> ##
> @@ -252,7 +254,7 @@
> '*bps_max': 'int', '*bps_rd_max': 'int',
> '*bps_wr_max': 'int', '*iops_max': 'int',
> '*iops_rd_max': 'int', '*iops_wr_max': 'int',
> -'*iops_size': 'int' } }
> +'*iops_size': 'int', 'cache': 'BlockdevCacheOptions' } }
>
> ##
> # @BlockDeviceIoStatus:
Before this patch, QAPI type BlockdevCacheOptions is used only as a
member of BlockdevOptionsBase.
BlockdevOptionsBase is a collection configuration settings.
Consequently, it's a complex type whose members are optional exactly
when the configuration setting is optional. Makes sense.
Complication: a few options are wrapped in another complex type,
BlockdevCacheOptions. It's optional, but that's not sufficient, it's
members are all optional, too.
BlockdevCacheOptions is the only complex member of BlockdevOptionsBase.
The others are all simple or enum.
In this patch, you reuse BlockdevCacheOptions for a purpose other than
configuration: *querying* configuration. In the query result, neither
the complex type nor its members are optional. The schema reflects the
former (the patch hunk has 'cache', not '*cache'), but not the latter.
Do we want the schema to reflect reality accurately?
If no, we should still document the places where it doesn't, like this
one.
If yes, how can we fix this particular inaccuracy?
The obvious solution is not to reuse BlockdevCacheOptions. Create a
second type, identical except for its members aren't optional.
Another idea is to add means to the schema to declare a member "deep
optional": not just the member is optional, but if it's present, its
members are optional, too. Begs the question whether its member's
member's are optional. Hmm.
Yet another idea is to declare nesting configuration options stupid, and
just not do it, but it may be too late for that.
Other ideas?
[...]
Re: [Qemu-devel] [PATCH v2] pci-hotplug-old: avoid lossing error message
writes: > From: Gonglei > > When scsi_bus_legacy_add_drive() produce error, > we will loss error message. Using error_report > report it. > > Cc: Markus Armbruster > Signed-off-by: Gonglei Reviewed-by: Markus Armbruster
Re: [Qemu-devel] qemu process stuck in Rl state
On Thu, Sep 18, 2014 at 2:49 PM, Stefan Hajnoczi wrote: > On Wed, Sep 17, 2014 at 11:56:57PM +0400, Andrey Korolyov wrote: >> I`ve faced an issue with qemu VMs with very large uptime spans - half >> of year or so. They are hanging in running state forever and are not >> killable in any imaginable fashion. Tried to freeze it via freezer cg >> without any luck. VM itself went unresponsive with zero cpu >> consumption after reaching 'forever running' point. >> >> I am going to reset the host in a couple of hours, so any timed ideas >> for debugging this state will be very appreciated. > > A couple of shots at figuring out what the process is doing: > > cat /proc/$PID/stack > cat /proc/$PID/syscall > gdb $PID > (gdb) thread apply all bt Thanks Stefan, of course any attempts to attach to the process or dump core failed at very beginning. I compared proc contents with live VM and found nothing suspicious. The question is about what I should try to do facing supposedly kernel bug, if no possibility to determine which code is currently executing by emulator is available. Also if it may help, both affected VMs on different hosts has a simular process uptime (from end of May). Just to repeat - the process is not reacting to any signal, have zero CPU consumption immediately after bug appearance and therefore cannot be stopped/frozen.
[Qemu-devel] [PATCH v2] qemu-char: Do not disconnect when there's data for reading
After commit 812c1057f6175ac9a9829fa2920a2b5783814193 (Handle G_IO_HUP
in tcp_chr_read for tcp chardev), connections are disconnected when in
G_IO_HUP condition.
However, it's possible that there is still data for reading in the channel.
In that case, the remaining data is not handled.
I saw a related bug when running socat in write-only mode, after
$ echo "quit" | socat -u - UNIX-CONNECT:qemu-monitor
the monitor won't not run the 'quit' command.
Instead of GIOCondition, this patch uses the return value of tcp_chr_recv()
to check the state of connection as suggested by Kirill.
Cc: Kirill Batuzov
Cc: Nikolay Nikolaev
Cc: Markus Armbruster
Cc: Anthony Liguori
Signed-off-by: Zifei Tong
---
qemu-char.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 2a3cb9f..6cc69fa 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2692,12 +2692,6 @@ static gboolean tcp_chr_read(GIOChannel *chan,
GIOCondition cond, void *opaque)
uint8_t buf[READ_BUF_LEN];
int len, size;
-if (cond & G_IO_HUP) {
-/* connection closed */
-tcp_chr_disconnect(chr);
-return TRUE;
-}
-
if (!s->connected || s->max_size <= 0) {
return TRUE;
}
@@ -2705,7 +2699,7 @@ static gboolean tcp_chr_read(GIOChannel *chan,
GIOCondition cond, void *opaque)
if (len > s->max_size)
len = s->max_size;
size = tcp_chr_recv(chr, (void *)buf, len);
-if (size == 0) {
+if (size == 0 || (size < 0 && !(errno == EAGAIN || errno == EINTR))) {
/* connection closed */
tcp_chr_disconnect(chr);
} else if (size > 0) {
--
2.1.0
[Qemu-devel] [PATCH v5 0/1] async: aio_context_new(): Handle event_notifier_init failure
v4->v5 -- * Set thread_id before calling aio_context_new(). That way qmp_query_iothreads() will display thread_id -1 for a failed IOThread object than an uninitialized value. * qemu_init_main_loop() will return -EMFILE if aio_context_new() fail. The actual failure reason is placed in the propagated error message. * Initialize 'Error *local_error' value to NULL. v3->v4 -- * Remove escaped single quotes from error messages. * Rephrase commit log. v2->v3 -- * Remove errno usage and print the detailed message based on errno when event_notifier_init() fails. * Propagate error and return from iothread_complete() if aio_context_new() fails. * Return if !iothread->ctx from iothread_instance_finalize(), used by QOM when object_unref(obj) is called after user_creatable_complete() fails. * Remove cosmetic fixes accidentally introduced by editor and fix code style issues. v1->v2 -- * aio_context_new() returns NULL if the initialization of event notifier fails. * Add descriptive error messages if aio_context_new() and event_notifier_init() fail. * Fix gpollfds leak. Chrysostomos Nanakos (1): async: aio_context_new(): Handle event_notifier_init failure async.c | 16 +++- include/block/aio.h |2 +- include/qemu/main-loop.h |2 +- iothread.c | 11 ++- main-loop.c |9 +++-- qemu-img.c |8 +++- qemu-io.c|7 ++- qemu-nbd.c |6 +- tests/test-aio.c | 10 +- tests/test-thread-pool.c | 10 +- tests/test-throttle.c| 10 +- vl.c |5 +++-- 12 files changed, 78 insertions(+), 18 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH v5 1/1] async: aio_context_new(): Handle event_notifier_init failure
On a system with a low limit of open files the initialization
of the event notifier could fail and QEMU exits without printing any
error information to the user.
The problem can be easily reproduced by enforcing a low limit of open
files and start QEMU with enough I/O threads to hit this limit.
The same problem raises, without the creation of I/O threads, while
QEMU initializes the main event loop by enforcing an even lower limit of
open files.
This commit adds an error message on failure:
# qemu [...] -object iothread,id=iothread0 -object iothread,id=iothread1
qemu: Failed to initialize event notifier: Too many open files in system
Signed-off-by: Chrysostomos Nanakos
---
async.c | 16 +++-
include/block/aio.h |2 +-
include/qemu/main-loop.h |2 +-
iothread.c | 11 ++-
main-loop.c |9 +++--
qemu-img.c |8 +++-
qemu-io.c|7 ++-
qemu-nbd.c |6 +-
tests/test-aio.c | 10 +-
tests/test-thread-pool.c | 10 +-
tests/test-throttle.c| 10 +-
vl.c |5 +++--
12 files changed, 78 insertions(+), 18 deletions(-)
diff --git a/async.c b/async.c
index a99e7f6..6e1b282 100644
--- a/async.c
+++ b/async.c
@@ -289,18 +289,24 @@ static void aio_rfifolock_cb(void *opaque)
aio_notify(opaque);
}
-AioContext *aio_context_new(void)
+AioContext *aio_context_new(Error **errp)
{
+int ret;
AioContext *ctx;
ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
+ret = event_notifier_init(&ctx->notifier, false);
+if (ret < 0) {
+g_source_destroy(&ctx->source);
+error_setg_errno(errp, -ret, "Failed to initialize event notifier");
+return NULL;
+}
+aio_set_event_notifier(ctx, &ctx->notifier,
+ (EventNotifierHandler *)
+ event_notifier_test_and_clear);
ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
ctx->thread_pool = NULL;
qemu_mutex_init(&ctx->bh_lock);
rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx);
-event_notifier_init(&ctx->notifier, false);
-aio_set_event_notifier(ctx, &ctx->notifier,
- (EventNotifierHandler *)
- event_notifier_test_and_clear);
timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);
return ctx;
diff --git a/include/block/aio.h b/include/block/aio.h
index 4603c0f..f3d5517 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -99,7 +99,7 @@ void aio_set_dispatching(AioContext *ctx, bool dispatching);
* They also provide bottom halves, a service to execute a piece of code
* as soon as possible.
*/
-AioContext *aio_context_new(void);
+AioContext *aio_context_new(Error **errp);
/**
* aio_context_ref:
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 6f0200a..62c68c0 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -42,7 +42,7 @@
*
* In the case of QEMU tools, this will also start/initialize timers.
*/
-int qemu_init_main_loop(void);
+int qemu_init_main_loop(Error **errp);
/**
* main_loop_wait: Run one iteration of the main loop.
diff --git a/iothread.c b/iothread.c
index d9403cf..342a23f 100644
--- a/iothread.c
+++ b/iothread.c
@@ -17,6 +17,7 @@
#include "block/aio.h"
#include "sysemu/iothread.h"
#include "qmp-commands.h"
+#include "qemu/error-report.h"
#define IOTHREADS_PATH "/objects"
@@ -53,6 +54,9 @@ static void iothread_instance_finalize(Object *obj)
{
IOThread *iothread = IOTHREAD(obj);
+if (!iothread->ctx) {
+return;
+}
iothread->stopping = true;
aio_notify(iothread->ctx);
qemu_thread_join(&iothread->thread);
@@ -63,11 +67,16 @@ static void iothread_instance_finalize(Object *obj)
static void iothread_complete(UserCreatable *obj, Error **errp)
{
+Error *local_error = NULL;
IOThread *iothread = IOTHREAD(obj);
iothread->stopping = false;
-iothread->ctx = aio_context_new();
iothread->thread_id = -1;
+iothread->ctx = aio_context_new(&local_error);
+if (!iothread->ctx) {
+error_propagate(errp, local_error);
+return;
+}
qemu_mutex_init(&iothread->init_done_lock);
qemu_cond_init(&iothread->init_done_cond);
diff --git a/main-loop.c b/main-loop.c
index 3cc79f8..53393a4 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -126,10 +126,11 @@ void qemu_notify_event(void)
static GArray *gpollfds;
-int qemu_init_main_loop(void)
+int qemu_init_main_loop(Error **errp)
{
int ret;
GSource *src;
+Error *local_error = NULL;
init_clocks();
@@ -138,8 +139,12 @@ int qemu_init_main_loop(void)
return ret;
}
+qemu_aio_context = aio_context_new(&local_error);
+if (!qemu_aio_context) {
+error_propagate(errp, local_error);
+return
Re: [Qemu-devel] [PATCH 1/6] block/qapi: Add cache information to query-block
Markus Armbruster writes: > Kevin Wolf writes: [...] >> @@ -335,6 +336,13 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) >> continue; >> } >> >> +inserted = info->value->inserted; >> + >> +monitor_printf(mon, "Cache mode: %s%s%s\n", >> + inserted->cache->writeback ? "writeback" : >> "writethrough", >> + inserted->cache->direct ? ", direct" : "", >> + inserted->cache->no_flush ? ", ignore flushes" : ""); >> + > > If !inserted->cache->writeback && inserted->cache->direct, this prints > "Cache mode: , writeback". Nevermind, I can't read. [...]
Re: [Qemu-devel] [INVITE] OVMF BoF session at the KVM Forum 2014
Hello Laszlo, Am 18.09.2014 um 10:23 schrieb Laszlo Ersek: > I've been made an offer that I couldn't refuse :) to "organize" a Birds > of a Feather session concerning OVMF at the KVM Forum 2014. > > Interested people, please sign up: > > http://www.linux-kvm.org/page/KVM_Forum_2014_BOF#OVMF Nice idea. Your summary mentions only ia32 and x86_64 - I would be interested in an update on OVMF for AArch64 - there seemed to already be support for ARM's Foundation Model but not yet for QEMU. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2] pci-hotplug-old: avoid lossing error message
> From: Markus Armbruster [mailto:arm...@redhat.com] > Sent: Thursday, September 18, 2014 7:09 PM > Subject: Re: [Qemu-devel] [PATCH v2] pci-hotplug-old: avoid lossing error > message > > writes: > > > From: Gonglei > > > > When scsi_bus_legacy_add_drive() produce error, > > we will loss error message. Using error_report > > report it. > > > > Cc: Markus Armbruster > > Signed-off-by: Gonglei > > Reviewed-by: Markus Armbruster Thanks :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 2/6] block: Add optional device argument to query-block
Kevin Wolf writes:
> This allows querying one specific BlockDriverState instead of a list of
> all drives.
I've long resisted optional arguments to limit list-valued queries,
because I believe the additional interface complexity isn't worth the
additional utility. But resistance might be futile.
We have one only query- with an optional limiting arguement[*]:
query-rx-filter.
If we go down this route, then I'd prefer to have such optional limiting
for all query- commands returning lists of elements describing some
aspect of a named object. Preference != demand.
>
> Signed-off-by: Kevin Wolf
> ---
> block/qapi.c | 15 +--
> hmp.c| 6 +++---
> qapi/block-core.json | 8 ++--
> 3 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 6b43bc3..2a060d3 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -367,13 +367,24 @@ static BlockStats *bdrv_query_stats(const
> BlockDriverState *bs)
> return s;
> }
>
> -BlockInfoList *qmp_query_block(Error **errp)
> +BlockInfoList *qmp_query_block(bool has_device, const char *device,
> + Error **errp)
> {
> BlockInfoList *head = NULL, **p_next = &head;
> BlockDriverState *bs = NULL;
> Error *local_err = NULL;
>
> - while ((bs = bdrv_next(bs))) {
> +if (has_device) {
> +bs = bdrv_find(device);
> +if (bs == NULL) {
> +error_setg(errp, "Device not found");
> +goto err;
> +}
> +} else {
> +bs = bdrv_next(NULL);
> +}
> +
> +for (; bs; (bs = has_device ? NULL : bdrv_next(bs))) {
> BlockInfoList *info = g_malloc0(sizeof(*info));
> bdrv_query_info(bs, &info->value, &local_err);
> if (local_err) {
I'd find
for (bs = bdrv_next(NULL); bs; bs == bdrv_next(bs)) {
if (device && strcmp(device, bs->device)) {
continue;
}
}
simpler and clearer. It's how qmp_query_rx_filter() works.
> diff --git a/hmp.c b/hmp.c
> index c3846b8..02eee80 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -298,7 +298,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
> const char *device = qdict_get_try_str(qdict, "device");
> bool verbose = qdict_get_try_bool(qdict, "verbose", 0);
>
> -block_list = qmp_query_block(NULL);
> +block_list = qmp_query_block(false, NULL, NULL);
>
> for (info = block_list; info; info = info->next) {
> if (device && strcmp(device, info->value->device)) {
> @@ -847,7 +847,7 @@ void hmp_cont(Monitor *mon, const QDict *qdict)
> BlockInfoList *bdev_list, *bdev;
> Error *err = NULL;
>
> -bdev_list = qmp_query_block(NULL);
> +bdev_list = qmp_query_block(false, NULL, NULL);
> for (bdev = bdev_list; bdev; bdev = bdev->next) {
> if (key_is_missing(bdev->value)) {
> monitor_read_block_device_key(mon, bdev->value->device,
> @@ -1588,7 +1588,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict
> *qdict)
> /* Then try adding all block devices. If one fails, close all and
> * exit.
> */
> -block_list = qmp_query_block(NULL);
> +block_list = qmp_query_block(false, NULL, NULL);
>
> for (info = block_list; info; info = info->next) {
> if (!info->value->has_inserted) {
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index c53b587..ddc3fe0 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -354,13 +354,17 @@
> ##
> # @query-block:
> #
> -# Get a list of BlockInfo for all virtual block devices.
> +# Get a list of BlockInfo for all or one specific virtual block device.
> +#
> +# @device: #optional The id of the block device to inspect. (Since 2.2)
> #
> # Returns: a list of @BlockInfo describing each virtual block device
> #
> # Since: 0.14.0
> ##
This is how we document query-rx-filter:
##
# @query-rx-filter:
#
# Return rx-filter information for all NICs (or for the given NIC).
#
# @name: #optional net client name
#
# Returns: list of @RxFilterInfo for all NICs (or for the given NIC).
# Returns an error if the given @name doesn't exist, or given
# NIC doesn't support rx-filter querying, or given net client
# isn't a NIC.
#
# Since: 1.6
##
I find that slightly clearer. Perhaps you'd like to steal from it.
> -{ 'command': 'query-block', 'returns': ['BlockInfo'] }
> +{ 'command': 'query-block',
> + 'data': { '*device': 'str' },
> + 'returns': ['BlockInfo'] }
>
> ##
> # @BlockDeviceStats:
[*] Yes, I objected to the argument, but I didn't insist. I won't
insist now.
Re: [Qemu-devel] [PATCH 1/6] block/qapi: Add cache information to query-block
Am 18.09.2014 um 13:04 hat Markus Armbruster geschrieben: > Before this patch, QAPI type BlockdevCacheOptions is used only as a > member of BlockdevOptionsBase. > > BlockdevOptionsBase is a collection configuration settings. > Consequently, it's a complex type whose members are optional exactly > when the configuration setting is optional. Makes sense. > > Complication: a few options are wrapped in another complex type, > BlockdevCacheOptions. It's optional, but that's not sufficient, it's > members are all optional, too. > > BlockdevCacheOptions is the only complex member of BlockdevOptionsBase. > The others are all simple or enum. > > In this patch, you reuse BlockdevCacheOptions for a purpose other than > configuration: *querying* configuration. In the query result, neither > the complex type nor its members are optional. The schema reflects the > former (the patch hunk has 'cache', not '*cache'), but not the latter. > > Do we want the schema to reflect reality accurately? > > If no, we should still document the places where it doesn't, like this > one. That's a fair requirement. If anything is optional in a return value, we should specify the conditions under which it is there or missing. Including, of course, if it's always or never there. > If yes, how can we fix this particular inaccuracy? > > The obvious solution is not to reuse BlockdevCacheOptions. Create a > second type, identical except for its members aren't optional. I can introduce a BlockdevCacheInfo instead. While I'm not completely excited about having two structs for each option dict (one for configuring, one for querying), there's precedence for this and it's at least a small struct this time. > Another idea is to add means to the schema to declare a member "deep > optional": not just the member is optional, but if it's present, its > members are optional, too. Begs the question whether its member's > member's are optional. Hmm. "deep optional" doesn't sound like it makes a lot of sense conceptually, even if it might happen to be a hack that gets us the right result in this one specific case. > Yet another idea is to declare nesting configuration options stupid, and > just not do it, but it may be too late for that. > > Other ideas? I think the choice is between adding BlockdevCacheInfo and changing documentation while reusing BlockdevCacheOptions. Both are fine with me. Which one do you prefer? Kevin
Re: [Qemu-devel] [PATCH 3/6] block: Introduce query-block-node
Kevin Wolf writes:
> This new command is a renamed version of query-named-block-nodes
> with an additional argument that allows querying only one specific
> BlockDriverState instead of getting a list for all of them.
>
> Signed-off-by: Kevin Wolf
> ---
> block.c | 15 +--
> blockdev.c| 8 +++-
> include/block/block.h | 2 +-
> qapi/block-core.json | 19 ---
> 4 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/block.c b/block.c
> index bcd952a..6c538d3 100644
> --- a/block.c
> +++ b/block.c
> @@ -3823,13 +3823,24 @@ BlockDriverState *bdrv_find_node(const char
> *node_name)
> }
>
> /* Put this QMP function here so it can access the static graph_bdrv_states.
> */
> -BlockDeviceInfoList *bdrv_named_nodes_list(void)
> +BlockDeviceInfoList *bdrv_named_nodes_list(const char *device, Error **errp)
> {
> BlockDeviceInfoList *list, *entry;
> BlockDriverState *bs;
> +Error *local_err = NULL;
> +
> +if (device) {
> +bs = bdrv_lookup_bs(device, device, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return NULL;
> +}
> +} else {
> +bs = QTAILQ_FIRST(&graph_bdrv_states);
> +}
>
> list = NULL;
> -QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
> +for (; bs; bs = device ? NULL : QTAILQ_NEXT(bs, node_list)) {
> entry = g_malloc0(sizeof(*entry));
> entry->value = bdrv_block_device_info(bs);
> entry->next = list;
Same loop as in PATCH 2/6, same suggestion to simplify it.
> diff --git a/blockdev.c b/blockdev.c
> index b361fbb..a194d04 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2127,9 +2127,15 @@ void qmp_drive_backup(const char *device, const char
> *target,
> }
> }
>
> +BlockDeviceInfoList *qmp_query_block_node(bool has_device, const char
> *device,
> + Error **errp)
> +{
> +return bdrv_named_nodes_list(has_device ? device : NULL, errp);
> +}
> +
> BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
> {
> -return bdrv_named_nodes_list();
> +return qmp_query_block_node(false, NULL, errp);
> }
>
> #define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
> diff --git a/include/block/block.h b/include/block/block.h
> index 07d6d8e..0caab0d 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -403,7 +403,7 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag);
> const char *bdrv_get_format_name(BlockDriverState *bs);
> BlockDriverState *bdrv_find(const char *name);
> BlockDriverState *bdrv_find_node(const char *node_name);
> -BlockDeviceInfoList *bdrv_named_nodes_list(void);
> +BlockDeviceInfoList *bdrv_named_nodes_list(const char *device, Error **errp);
> BlockDriverState *bdrv_lookup_bs(const char *device,
> const char *node_name,
> Error **errp);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ddc3fe0..f170c7e 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -797,13 +797,26 @@
> ##
> # @query-named-block-nodes
> #
> -# Get the named block driver list
> +# Deprecated, may be removed in future versions. Use query-block-node
> instead.
> +#
> +# Since 2.0, until 2.1
Suggest
##
# @query-named-block-nodes
#
# Get the named block driver list
#
# Since 2.0
+# Deprecated since 2.1, use query-block-node instead
and a general understanding that "deprecated" implies "should not be
used anymore" and "may be removed in a future release".
> +##
> +{ 'command': 'query-named-block-nodes', 'returns': [ 'BlockDeviceInfo' ] }
> +
> +##
> +# @query-block-node
> +#
> +# Gets information about one or all block device nodes.
> +#
> +# @device: #optional The id or node-name of the block device to inspect.
> #
> # Returns: the list of BlockDeviceInfo
> #
> -# Since 2.0
> +# Since 2.2
> ##
> -{ 'command': 'query-named-block-nodes', 'returns': [ 'BlockDeviceInfo' ] }
> +{ 'command': 'query-block-node',
> + 'data': { '*device': 'str' },
> + 'returns': [ 'BlockDeviceInfo' ] }
>
> ##
> # @drive-mirror
[Qemu-devel] [PATCH v2] pc-dimm/numa: Fix stat of memory size in node when hotplug memory
When do memory hotplug, if there is numa node, we should add
the memory size to the corresponding node memory size.
For now, it mainly affects the result of hmp command "info numa".
Signed-off-by: zhanghailiang
---
v2:
- Don't modify the numa_info.node_mem directly when treating hotplug memory,
fix the "info numa" instead (suggested by Igor Mammedov)
---
hw/mem/pc-dimm.c | 27 +++
include/hw/mem/pc-dimm.h | 2 ++
include/sysemu/sysemu.h | 1 +
monitor.c| 6 +-
numa.c | 15 +++
5 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 5bfc5b7..7b233a6 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -195,6 +195,33 @@ out:
return ret;
}
+static int pc_dimm_stat_mem_size(Object *obj, void *opaque)
+{
+uint64_t *node_mem = opaque;
+int ret;
+
+if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+DeviceState *dev = DEVICE(obj);
+if (dev->realized && dev->hotplugged) {
+PCDIMMDevice *dimm = PC_DIMM(obj);
+int size = object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
+ NULL);
+if (size < 0) {
+return -1;
+}
+node_mem[dimm->node] += size;
+}
+}
+
+ret = object_child_foreach(obj, pc_dimm_stat_mem_size, opaque);
+return ret;
+}
+
+void pc_dimm_stat_node_mem(uint64_t *node_mem)
+{
+object_child_foreach(qdev_get_machine(), pc_dimm_stat_mem_size, node_mem);
+}
+
static Property pc_dimm_properties[] = {
DEFINE_PROP_UINT64(PC_DIMM_ADDR_PROP, PCDIMMDevice, addr, 0),
DEFINE_PROP_UINT32(PC_DIMM_NODE_PROP, PCDIMMDevice, node, 0),
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 761eeef..0c9a8eb 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -78,4 +78,6 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
int qmp_pc_dimm_device_list(Object *obj, void *opaque);
+
+void pc_dimm_stat_node_mem(uint64_t *node_mem);
#endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..cfc1592 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -160,6 +160,7 @@ typedef struct node_info {
extern NodeInfo numa_info[MAX_NODES];
void set_numa_nodes(void);
void set_numa_modes(void);
+int query_numa_node_mem(uint64_t *node_mem);
extern QemuOptsList qemu_numa_opts;
int numa_init_func(QemuOpts *opts, void *opaque);
diff --git a/monitor.c b/monitor.c
index 7467521..c8c812f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1948,7 +1948,10 @@ static void do_info_numa(Monitor *mon, const QDict
*qdict)
{
int i;
CPUState *cpu;
+uint64_t *node_mem;
+node_mem = g_new0(uint64_t, nb_numa_nodes);
+query_numa_node_mem(node_mem);
monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
for (i = 0; i < nb_numa_nodes; i++) {
monitor_printf(mon, "node %d cpus:", i);
@@ -1959,8 +1962,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
}
monitor_printf(mon, "\n");
monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
-numa_info[i].node_mem >> 20);
+node_mem[i] >> 20);
}
+g_free(node_mem);
}
#ifdef CONFIG_PROFILER
diff --git a/numa.c b/numa.c
index 3b98135..4e27dd8 100644
--- a/numa.c
+++ b/numa.c
@@ -35,6 +35,7 @@
#include "hw/boards.h"
#include "sysemu/hostmem.h"
#include "qmp-commands.h"
+#include "hw/mem/pc-dimm.h"
QemuOptsList qemu_numa_opts = {
.name = "numa",
@@ -315,6 +316,20 @@ void memory_region_allocate_system_memory(MemoryRegion
*mr, Object *owner,
}
}
+int query_numa_node_mem(uint64_t *node_mem)
+{
+int i;
+
+if (nb_numa_nodes <= 0) {
+return 0;
+}
+pc_dimm_stat_node_mem(node_mem);
+for (i = 0; i < nb_numa_nodes; i++) {
+node_mem[i] += numa_info[i].node_mem;
+}
+return 0;
+}
+
static int query_memdev(Object *obj, void *opaque)
{
MemdevList **list = opaque;
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2] qemu-char: Do not disconnect when there's data for reading
Zifei Tong writes:
> After commit 812c1057f6175ac9a9829fa2920a2b5783814193 (Handle G_IO_HUP
> in tcp_chr_read for tcp chardev), connections are disconnected when in
> G_IO_HUP condition.
>
> However, it's possible that there is still data for reading in the channel.
> In that case, the remaining data is not handled.
>
> I saw a related bug when running socat in write-only mode, after
>
> $ echo "quit" | socat -u - UNIX-CONNECT:qemu-monitor
>
> the monitor won't not run the 'quit' command.
>
> Instead of GIOCondition, this patch uses the return value of tcp_chr_recv()
> to check the state of connection as suggested by Kirill.
>
> Cc: Kirill Batuzov
> Cc: Nikolay Nikolaev
> Cc: Markus Armbruster
> Cc: Anthony Liguori
> Signed-off-by: Zifei Tong
> ---
> qemu-char.c | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 2a3cb9f..6cc69fa 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2692,12 +2692,6 @@ static gboolean tcp_chr_read(GIOChannel *chan,
> GIOCondition cond, void *opaque)
> uint8_t buf[READ_BUF_LEN];
> int len, size;
>
> -if (cond & G_IO_HUP) {
> -/* connection closed */
> -tcp_chr_disconnect(chr);
> -return TRUE;
> -}
> -
> if (!s->connected || s->max_size <= 0) {
> return TRUE;
> }
> @@ -2705,7 +2699,7 @@ static gboolean tcp_chr_read(GIOChannel *chan,
> GIOCondition cond, void *opaque)
> if (len > s->max_size)
> len = s->max_size;
> size = tcp_chr_recv(chr, (void *)buf, len);
> -if (size == 0) {
> +if (size == 0 || (size < 0 && !(errno == EAGAIN || errno == EINTR))) {
> /* connection closed */
> tcp_chr_disconnect(chr);
> } else if (size > 0) {
What about EWOULDBLOCK?
The /* connection closed */ comment is now more inaccurate :)
If size == 0, the peer has shut down its sending end of the connection.
If size < 0, our attempt to receive encountered an error.
I'd just drop the comment.
Re: [Qemu-devel] [PATCH] qga: Fix possible freed memory accessing
On 09/17/2014 09:33 PM, zhanghailiang wrote:
> If readdir_r fails, error_setg_errno will reference the freed
> pointer *dirpath*.
>
> Signed-off-by: zhanghailiang
> ---
> qga/commands-posix.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
> for (;;) {
> if (readdir_r(dir, &entry, &result) != 0) {
Eww. We're using readdir_r? That's an inherently broken interface,
which can risk buffer overflow. readdir should be preferred.
http://austingroupbugs.net/view.php?id=696
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [INVITE] OVMF BoF session at the KVM Forum 2014
On 09/18/14 13:44, Andreas Färber wrote: > Hello Laszlo, > > Am 18.09.2014 um 10:23 schrieb Laszlo Ersek: >> I've been made an offer that I couldn't refuse :) to "organize" a Birds >> of a Feather session concerning OVMF at the KVM Forum 2014. >> >> Interested people, please sign up: >> >> http://www.linux-kvm.org/page/KVM_Forum_2014_BOF#OVMF > > Nice idea. Your summary mentions only ia32 and x86_64 - I would be > interested in an update on OVMF for AArch64 - there seemed to already be > support for ARM's Foundation Model but not yet for QEMU. We've successfully UEFI-booted - GNU/Linux guest(s) on - upstream edk2 (*) and - upstream qemu-system-aarch64 with - TCG on x86_64 host, - KVM on aarch64 host (**) (*) Ard's patches for upstream edk2 are in the process of being tested / merged. (**) Ard's patches for the upstream host kernel (== KVM) have been... ugh, not sure... applied to a maintainer tree? Ard? :) So, it works (as far as I tested it myself on TCG, and heard reports about it on KVM), but right now you need to apply a handful of pending patches manually. We can certainly talk about Aarch64 at the BoF, but then Ard should co-organize. No good deed goes unpunished, as ever! :) Cheers, Laszlo
Re: [Qemu-devel] [PATCH] pci-hotplug-old: avoid lossing error message
On 09/18/2014 02:53 AM, arei.gong...@huawei.com wrote: > From: Gonglei s/lossing/losing/ in the subject. > > When scsi_bus_legacy_add_drive() produce error, s/produce error/produces an error/ > we will loss error message. Using error_report s/loss/lose the/ s/Using error_report/Use error_report to/ > report it. > > Cc: Markus Armbruster > Signed-off-by: Gonglei > --- > hw/pci/pci-hotplug-old.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > With the commit message improved, Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block: Catch simultaneous usage of options and their aliases
On 09/18/2014 03:57 AM, Kevin Wolf wrote:
> While thinking about precedence of conflicting block device options from
> different sources, I noticed that you can specify both an option and its
> legacy alias at the same time (e.g. readonly=on,read-only=off). Rather
> than specifying the order of precedence, we should simply forbid such
> combinations.
>
> Signed-off-by: Kevin Wolf
> ---
> blockdev.c | 46
> +++---
> tests/qemu-iotests/051 | 23 +++
> tests/qemu-iotests/051.out | 45 +
> 3 files changed, 99 insertions(+), 15 deletions(-)
Reviewed-by: Eric Blake
>
> -static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to)
> +static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to,
> +Error **errp)
> {
> const char *value;
>
> +if (*errp) {
> +return;
> +}
Not the most typical usage, so it might be worth a comment that this
function can be called with errp already set. But since it's static,
it's not too hard to figure out as-is.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] pci-hotplug-old: avoid lossing error message
> -Original Message- > From: Eric Blake [mailto:ebl...@redhat.com] > Sent: Thursday, September 18, 2014 8:22 PM > To: Gonglei (Arei); qemu-devel@nongnu.org > Cc: Huangweidong (C); arm...@redhat.com; m...@redhat.com > Subject: Re: [Qemu-devel] [PATCH] pci-hotplug-old: avoid lossing error message > > On 09/18/2014 02:53 AM, arei.gong...@huawei.com wrote: > > From: Gonglei > > s/lossing/losing/ in the subject. > > > > > When scsi_bus_legacy_add_drive() produce error, > > s/produce error/produces an error/ > > > we will loss error message. Using error_report > > s/loss/lose the/ > s/Using error_report/Use error_report to/ > > > report it. > > > > Cc: Markus Armbruster > > Signed-off-by: Gonglei > > --- > > hw/pci/pci-hotplug-old.c | 7 +-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > With the commit message improved, > Reviewed-by: Eric Blake > Thanks, Eric. I posted v2 and Markus have reviewed. If you haven't dissidence for v2, I will post v3 by your comments and add your both 'R-b' tag. :) Best regards, -Gonglei > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v2] pci-hotplug-old: avoid lossing error message
On 09/18/2014 03:13 AM, arei.gong...@huawei.com wrote: > From: Gonglei > > When scsi_bus_legacy_add_drive() produce error, > we will loss error message. Using error_report > report it. Same typos/grammar issues as I flagged on v1. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] vhost-user: VHOST_SET_MEM_TABLE, VHOST_SET_VRING_CALL need a reply?
On 2014/9/18 13:16, Michael S. Tsirkin wrote: > On Thu, Sep 18, 2014 at 08:45:37AM +0800, Linhaifeng wrote: >> >> >> On 2014/9/17 17:56, Michael S. Tsirkin wrote: >>> On Wed, Sep 17, 2014 at 05:39:04PM +0800, Linhaifeng wrote: I think maybe is not need for the backend to wait for response. There is another way.vhost-user send "VHOST_GET_MEM_TABLE" to qemu then qemu send VHOST_SET_MEM_TABLE to update the regions of vhost-user.same as other command. If qemu could response the request of the vhost-user.the vhost-user could update date at anytime. >>> >>> The updates are initiated by QEMU, as a result of IOMMU, >>> memory hotplug or some other configuration change. >> >> How to deal with the vhost-user restart? >> when vhost-user restart it will lost the infomation which QEMU send. >> >> In the kernel mode vhost will restart with QEMU but in the user mode vhost >> will not. > > vhost-user must restart with qemu only. > Sometimes qemu not allowed to restart. e.g. The customer want to update the vhost-user to a newer version but don't want to restart the VM. > The nature of virtio protocol is such that there's not enough in-memory > state for host to gracefully recover from losing VQ state. > > We could add a new feature to allow recovery by reporting > failure to guest, and disabling processing new requests. > Guest could respond by recovering / discarding submitted > requests, re-enabling the device (likely by executing a reset) > and re-submitting requests. > > > This would need a new feature bit though, and would have to > be acknowledged by guest. As such this would have to be > virtio 1.0 feature, virtio 0.x is frozen. > >>> I think it's very useful for Commercialization. On 2014/9/17 16:38, Michael S. Tsirkin wrote: > Reply-To: > > Thinking about the vhost-user protocol, VHOST_SET_MEM_TABLE > is used to update the memory mappings. > > So shouldn't we want for response? > Otherwise e.g. guest can start using the memory > that vhost-user can't access. > > Similarly, with an IOMMU vhost-user might access memory it shouldn't. > > VHOST_SET_VRING_CALL is used for MSI-X masking. > Again, after vector is masted by switching the call fd, > backend shouldn't assert the old one. > > Thoughts? > > >>> >>> . >>> > > . >
Re: [Qemu-devel] [PATCH] qga: Fix possible freed memory accessing
On 2014/9/18 20:17, Eric Blake wrote:
On 09/17/2014 09:33 PM, zhanghailiang wrote:
If readdir_r fails, error_setg_errno will reference the freed
pointer *dirpath*.
Signed-off-by: zhanghailiang
---
qga/commands-posix.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
for (;;) {
if (readdir_r(dir, &entry, &result) != 0) {
Eww. We're using readdir_r? That's an inherently broken interface,
which can risk buffer overflow. readdir should be preferred.
http://austingroupbugs.net/view.php?id=696
Yes, it is! Should i fix it in this patch together?;)
Thanks,
zhanghailiang
Re: [Qemu-devel] [PATCH 1/6] block/qapi: Add cache information to query-block
Kevin Wolf writes: > Am 18.09.2014 um 13:04 hat Markus Armbruster geschrieben: >> Before this patch, QAPI type BlockdevCacheOptions is used only as a >> member of BlockdevOptionsBase. >> >> BlockdevOptionsBase is a collection configuration settings. >> Consequently, it's a complex type whose members are optional exactly >> when the configuration setting is optional. Makes sense. >> >> Complication: a few options are wrapped in another complex type, >> BlockdevCacheOptions. It's optional, but that's not sufficient, it's >> members are all optional, too. >> >> BlockdevCacheOptions is the only complex member of BlockdevOptionsBase. >> The others are all simple or enum. >> >> In this patch, you reuse BlockdevCacheOptions for a purpose other than >> configuration: *querying* configuration. In the query result, neither >> the complex type nor its members are optional. The schema reflects the >> former (the patch hunk has 'cache', not '*cache'), but not the latter. >> >> Do we want the schema to reflect reality accurately? >> >> If no, we should still document the places where it doesn't, like this >> one. > > That's a fair requirement. If anything is optional in a return value, we > should specify the conditions under which it is there or missing. > Including, of course, if it's always or never there. Writing documentation saying an optional member is in fact not optional feels weird, and saying it's never there feels even weirder :) >> If yes, how can we fix this particular inaccuracy? >> >> The obvious solution is not to reuse BlockdevCacheOptions. Create a >> second type, identical except for its members aren't optional. > > I can introduce a BlockdevCacheInfo instead. While I'm not completely > excited about having two structs for each option dict (one for > configuring, one for querying), there's precedence for this and it's at > least a small struct this time. I'm not excited about the additional types, either. >> Another idea is to add means to the schema to declare a member "deep >> optional": not just the member is optional, but if it's present, its >> members are optional, too. Begs the question whether its member's >> member's are optional. Hmm. > > "deep optional" doesn't sound like it makes a lot of sense conceptually, > even if it might happen to be a hack that gets us the right result in > this one specific case. > >> Yet another idea is to declare nesting configuration options stupid, and >> just not do it, but it may be too late for that. >> >> Other ideas? > > I think the choice is between adding BlockdevCacheInfo and changing > documentation while reusing BlockdevCacheOptions. Both are fine with me. > Which one do you prefer? I'm leaning towards adding BlockdevCacheInfo, because you say there's precedence, and because I like the schema to be accurate more than I dislike the additional type.
[Qemu-devel] [PATCH v2 03/19] usb-net: convert init to realize
From: Gonglei
meanwhile, qerror_report_err() is a transitional interface to
help with converting existing HMP commands to QMP. It should
not be used elsewhere.
Signed-off-by: Gonglei
Reviewed-by: Paolo Bonzini
---
hw/usb/dev-network.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 518d536..686bd69 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -27,7 +27,7 @@
#include "hw/usb.h"
#include "hw/usb/desc.h"
#include "net/net.h"
-#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
#include "qemu/queue.h"
#include "qemu/config-file.h"
#include "sysemu/sysemu.h"
@@ -1341,7 +1341,7 @@ static NetClientInfo net_usbnet_info = {
.cleanup = usbnet_cleanup,
};
-static int usb_net_initfn(USBDevice *dev)
+static void usb_net_realize(USBDevice *dev, Error **errrp)
{
USBNetState *s = DO_UPCAST(USBNetState, dev, dev);
@@ -1373,7 +1373,6 @@ static int usb_net_initfn(USBDevice *dev)
usb_desc_set_string(dev, STRING_ETHADDR, s->usbstring_mac);
add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0");
-return 0;
}
static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
@@ -1392,7 +1391,7 @@ static USBDevice *usb_net_init(USBBus *bus, const char
*cmdline)
idx = net_client_init(opts, 0, &local_err);
if (local_err) {
-qerror_report_err(local_err);
+error_report("%s", error_get_pretty(local_err));
error_free(local_err);
return NULL;
}
@@ -1421,7 +1420,7 @@ static void usb_net_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_net_initfn;
+uc->realize = usb_net_realize;
uc->product_desc = "QEMU USB Network Interface";
uc->usb_desc = &desc_net;
uc->handle_reset = usb_net_handle_reset;
--
1.7.12.4
[Qemu-devel] [PATCH v2 01/19] usb-storage: fix possible memory leak and missing error message
From: Gonglei
When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
be not NULL, which will casue memory leak and missing error message.
Cc: Markus Armbruster
Signed-off-by: Gonglei
Reviewed-by: Paolo Bonzini
---
hw/usb/dev-storage.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae4efcb..f731b0a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
s->conf.bootindex, dev->serial,
&err);
if (!scsi_dev) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
return -1;
}
s->bus.qbus.allow_hotplug = 0;
--
1.7.12.4
[Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize
From: Gonglei DeviceClass->init is the old interface, let's convert usb devices to the new realize API. In this way, all the implementations now use error_setg instead of qerror_report/error_report for reporting error. Cc: Markus Armbruster Cc: Paolo Bonzini Cc: Gerd Hoffmann v2 -> v1: - fix PATCH 2, using qerror_report_err print error messages when attach fails (Paolo) - using errp instead of qerror_report_err introduced by fix 1 in PATCH 12 (Paolo) - fix missing return in PATCH 14 (Paolo) - add 'Reviewed-by' tag for other patches Thanks a lot for reviewing! Gonglei (19): usb-storage: fix possible memory leak and missing error message usb-bus: convert USBDeviceClass init to realize usb-net: convert init to realize libusb: convert init to realize libusb: using error_report instead of fprintf usb-hub: convert init to realize dev-storage: convert init to realize dev-storage: usring error_report instead of fprintf/printf dev-uas: convert init to realize dev-uas: using error_report instead of fprintf dev-bluetooth: convert init to realize dev-serial: convert init to realize usb-ccid: convert init to realize dev-hid: convert init to realize dev-wacom: convert init to realize usb-audio: convert init to realize usb-redir: convert init to realize usb-mtp: convert init to realize usb-bus: remove "init" from USBDeviceClass struct hw/usb/bus.c | 79 ++- hw/usb/dev-audio.c| 5 ++- hw/usb/dev-bluetooth.c| 6 ++-- hw/usb/dev-hid.c | 27 +++ hw/usb/dev-hub.c | 9 +++-- hw/usb/dev-mtp.c | 5 ++- hw/usb/dev-network.c | 9 +++-- hw/usb/dev-serial.c | 22 +++- hw/usb/dev-smartcard-reader.c | 5 ++- hw/usb/dev-storage.c | 42 --- hw/usb/dev-uas.c | 17 +- hw/usb/dev-wacom.c| 5 ++- hw/usb/host-libusb.c | 33 +- hw/usb/redirect.c | 21 +++- include/hw/usb.h | 10 -- 15 files changed, 150 insertions(+), 145 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH v2 04/19] libusb: convert init to realize
From: Gonglei
In this way, all the implementations now use
error_setg instead of error_report for reporting error.
Signed-off-by: Gonglei
Reviewed-by: Paolo Bonzini
---
hw/usb/host-libusb.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 9f92705..863be64 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -951,21 +951,21 @@ static void usb_host_exit_notifier(struct Notifier *n,
void *data)
}
}
-static int usb_host_initfn(USBDevice *udev)
+static void usb_host_realize(USBDevice *udev, Error **errp)
{
USBHostDevice *s = USB_HOST_DEVICE(udev);
if (s->match.vendor_id > 0x) {
-error_report("vendorid out of range");
-return -1;
+error_setg(errp, "vendorid out of range");
+return;
}
if (s->match.product_id > 0x) {
-error_report("productid out of range");
-return -1;
+error_setg(errp, "productid out of range");
+return;
}
if (s->match.addr > 127) {
-error_report("hostaddr out of range");
-return -1;
+error_setg(errp, "hostaddr out of range");
+return;
}
loglevel = s->loglevel;
@@ -980,7 +980,6 @@ static int usb_host_initfn(USBDevice *udev)
QTAILQ_INSERT_TAIL(&hostdevs, s, next);
add_boot_device_path(s->bootindex, &udev->qdev, NULL);
usb_host_auto_check(NULL);
-return 0;
}
static void usb_host_handle_destroy(USBDevice *udev)
@@ -1480,7 +1479,7 @@ static void usb_host_class_initfn(ObjectClass *klass,
void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_host_initfn;
+uc->realize = usb_host_realize;
uc->product_desc = "USB Host Device";
uc->cancel_packet = usb_host_cancel_packet;
uc->handle_data= usb_host_handle_data;
--
1.7.12.4
[Qemu-devel] [PATCH v2 09/19] dev-uas: convert init to realize
From: Gonglei
Signed-off-by: Gonglei
Reviewed-by: Paolo Bonzini
---
hw/usb/dev-uas.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 9832385..884c003 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -892,7 +892,7 @@ static void usb_uas_handle_destroy(USBDevice *dev)
qemu_bh_delete(uas->status_bh);
}
-static int usb_uas_init(USBDevice *dev)
+static void usb_uas_realize(USBDevice *dev, Error **errp)
{
UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
@@ -905,8 +905,6 @@ static int usb_uas_init(USBDevice *dev)
scsi_bus_new(&uas->bus, sizeof(uas->bus), DEVICE(dev),
&usb_uas_scsi_info, NULL);
-
-return 0;
}
static const VMStateDescription vmstate_usb_uas = {
@@ -928,7 +926,7 @@ static void usb_uas_class_initfn(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-uc->init = usb_uas_init;
+uc->realize = usb_uas_realize;
uc->product_desc = desc_strings[STR_PRODUCT];
uc->usb_desc = &desc;
uc->cancel_packet = usb_uas_cancel_io;
--
1.7.12.4
[Qemu-devel] [PATCH v2 02/19] usb-bus: convert USBDeviceClass init to realize
From: Gonglei
Add "realize/unrealize" in USBDeviceClass, which has errp
as a parameter. So all the implementations now use
error_setg instead of error_report for reporting error.
Note: this patch still keep "init" in USBDeviceClass, and
call kclass->init in usb_device_realize(), avoid breaking
git bisect. After realize all usb devices, will be removed.
Signed-off-by: Gonglei
---
hw/usb/bus.c | 81 +++-
hw/usb/dev-serial.c | 16 +--
hw/usb/dev-storage.c | 11 +--
hw/usb/host-libusb.c | 7 +++--
hw/usb/redirect.c| 6 +++-
include/hw/usb.h | 10 +--
6 files changed, 81 insertions(+), 50 deletions(-)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index c7c4dad..12881cb 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -9,7 +9,7 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev,
int indent);
static char *usb_get_dev_path(DeviceState *dev);
static char *usb_get_fw_dev_path(DeviceState *qdev);
-static int usb_qdev_exit(DeviceState *qdev);
+static void usb_qdev_unrealize(DeviceState *qdev, Error **errp);
static Property usb_props[] = {
DEFINE_PROP_STRING("port", USBDevice, port_path),
@@ -107,13 +107,15 @@ USBBus *usb_bus_find(int busnr)
return NULL;
}
-static int usb_device_init(USBDevice *dev)
+static void usb_device_realize(USBDevice *dev, Error **errp)
{
USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
-if (klass->init) {
-return klass->init(dev);
+
+if (klass->realize) {
+klass->realize(dev, errp);
+} else if (klass->init) {
+klass->init(dev);
}
-return 0;
}
USBDevice *usb_device_find_device(USBDevice *dev, uint8_t addr)
@@ -232,36 +234,41 @@ void usb_device_free_streams(USBDevice *dev, USBEndpoint
**eps, int nr_eps)
}
}
-static int usb_qdev_init(DeviceState *qdev)
+static void usb_qdev_realize(DeviceState *qdev, Error **errp)
{
USBDevice *dev = USB_DEVICE(qdev);
-int rc;
+Error *local_err = NULL;
pstrcpy(dev->product_desc, sizeof(dev->product_desc),
usb_device_get_product_desc(dev));
dev->auto_attach = 1;
QLIST_INIT(&dev->strings);
usb_ep_init(dev);
-rc = usb_claim_port(dev);
-if (rc != 0) {
-return rc;
+
+usb_claim_port(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
}
-rc = usb_device_init(dev);
-if (rc != 0) {
+
+usb_device_realize(dev, &local_err);
+if (local_err) {
usb_release_port(dev);
-return rc;
+error_propagate(errp, local_err);
+return;
}
+
if (dev->auto_attach) {
-rc = usb_device_attach(dev);
-if (rc != 0) {
-usb_qdev_exit(qdev);
-return rc;
+usb_device_attach(dev, &local_err);
+if (local_err) {
+usb_qdev_unrealize(qdev, NULL);
+error_propagate(errp, local_err);
+return;
}
}
-return 0;
}
-static int usb_qdev_exit(DeviceState *qdev)
+static void usb_qdev_unrealize(DeviceState *qdev, Error **errp)
{
USBDevice *dev = USB_DEVICE(qdev);
@@ -272,7 +279,6 @@ static int usb_qdev_exit(DeviceState *qdev)
if (dev->port) {
usb_release_port(dev);
}
-return 0;
}
typedef struct LegacyUSBFactory
@@ -392,7 +398,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
bus->nfree--;
}
-int usb_claim_port(USBDevice *dev)
+void usb_claim_port(USBDevice *dev, Error **errp)
{
USBBus *bus = usb_bus_from_device(dev);
USBPort *port;
@@ -406,9 +412,9 @@ int usb_claim_port(USBDevice *dev)
}
}
if (port == NULL) {
-error_report("Error: usb port %s (bus %s) not found (in use?)",
- dev->port_path, bus->qbus.name);
-return -1;
+error_setg(errp, "Error: usb port %s (bus %s) not found (in use?)",
+ dev->port_path, bus->qbus.name);
+return;
}
} else {
if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)),
"usb-hub") != 0) {
@@ -416,9 +422,9 @@ int usb_claim_port(USBDevice *dev)
usb_create_simple(bus, "usb-hub");
}
if (bus->nfree == 0) {
-error_report("Error: tried to attach usb device %s to a bus "
- "with no free ports", dev->product_desc);
-return -1;
+error_setg(errp, "Error: tried to attach usb device %s to a bus "
+ "with no free ports", dev->product_desc);
+return;
}
port = QTAILQ_FIRST(&bus->free);
}
@@ -432,7 +438,6 @@ int usb_claim_port(USBDevice *dev)
QTAILQ_INSERT_TAIL(&bus->used, port, next);
bus->nused++;
-return 0;
}
void usb_release_port(USBDevice *dev)
@@ -475,7 +480,7 @@ static void usb_mask_to_str(char *dest, size_t size,
}
}
-int usb_device_attach(USBDevice *
