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: 1830 bytes --]



On 07/27/2016 04:37 PM, Max Reitz wrote:
>> 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?
> drive_add is an HMP command. There's no other way for it to emit errors.
> 
> Strictly speaking, HMP commands are not supposed to be used by
> management applications like libvirt (the "H" stands for "human", after
> all). QMP's "human-monitor-command" is just a workaround because there
> are some HMP commands for which we do not have fully working QMP
> replacements yet. One such example is indeed drive_add, because as
> Markus correctly pointed out, blockdev-add is still considered experimental.
> 
> So we're still in the awkward spot of only having a legacy command
> (drive_add) and an experimental command (blockdev-add); and we have been
> in that spot for quite a while now (more than two years, I think). I
> think we're getting rather close to getting blockdev-add stable, but
> then again I'm afraid that might be something we've been thinking for
> the past two years.
> 
> Max
> 

My primary concern was the function drive_new which does not
propagate/report errors adequately under certain conditions, and
although there are multiple usages of drive_new after looking into them
I'm getting convinced that they are OK -- besides the one pointed
out here.

I read your comment as: "We are already working on this. It is best for
you to ignore the problem.". Is my reading correct? If it is I
can live with that.

Thanks for the explanations.

Cheers,
Halil


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