Re: [Qemu-devel] [PATCH v2 1/1] block: improve error handling in raw_open

[Halil Pasic <pasic@linux.vnet.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2591 bytes --]



On 07/26/2016 07:47 PM, Max Reitz wrote:
>> Frankly, I'm a bit uncomfortable with asking (do not want to be pushy),
>> > but do you have an opinion on the 'error_report_err' issue (pointed
>> > out in the cover letter part)?
> You are using drive_add with QMP? As far as I know, one can only do so
> with human-monitor-command which returns the error string like so:
> 
> {"return": "Could not open file: No such file or directory\r\n"}
> 

Yes, libvirt uses human-monitor-command, but are you sure the error
message is propagated back as described above?

Here is the call chain I'm talking about:

#0  error_report_err (err=0x1329a9d0) at /mnt/devel/root/git/qemu/util/error.c:228
#1  0x00000000100de4fe in drive_new (all_opts=all_opts@entry=0x12044010, block_default_type=<optimized out>) at /mnt/devel/root/git/qemu/blockdev.c:1134
#2  0x00000000100e47be in add_init_drive (optstr=<optimized out>) at /mnt/devel/root/git/qemu/device-hotplug.c:46
#3  hmp_drive_add (mon=0x3ffe3f7d188, qdict=0x120b3af0) at /mnt/devel/root/git/qemu/device-hotplug.c:66
#4  0x0000000010052092 in handle_hmp_command (mon=mon@entry=0x3ffe3f7d188, cmdline=0x110399ba "dummy file=/dev/sg924,if=none,id=drive-hostdev912", cmdline@entry=0x110399b0 "drive_add dummy file=/dev/sg924,if=none,id=drive-hostdev912")
    at /mnt/devel/root/git/qemu/monitor.c:2929
#5  0x0000000010052176 in qmp_human_monitor_command (command_line=0x110399b0 "drive_add dummy file=/dev/sg924,if=none,id=drive-hostdev912", has_cpu_index=<optimized out>, cpu_index=0, errp=errp@entry=0x3ffe3f7d310)
    at /mnt/devel/root/git/qemu/monitor.c:668
#6  0x00000000100f6faa in qmp_marshal_human_monitor_command (args=<optimized out>, ret=0x3ffe3f7d400, errp=0x3ffe3f7d3f8) at qmp-marshal.c:1777
[...]

Now if you examine #1 drive_new(all_opts,block_default_type) you see,
there is no errp argument and if you examine the code you see that the
error from blockdev_init which gets propagated properly to this point
gets "handled" by error_report_err (in QMP context! so does not much
good on this code path). AFAIU this can not work. Or am I wrong?


> Apart from that, as for QMP, in theory you're supposed to use
> blockdev-add and device_add, I think (although blockdev-add is still
> considered experimental...). And blockdev-add will return the error
> string like so:
> 
> {"error": {"class": "GenericError", "desc": "Could not open file: No
> such file or directory"}}
> 

I guess libvirt will eventually pick up blockdev-add so the problem
should eventually go away.

Cheers,
Halil




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]