Re: [Qemu-devel] [V2 2/2]Qemu: Add commands "hostcache_set" and "hostcache_get"

[Stefan Hajnoczi]

On Mon, May 23, 2011 at 8:04 AM, Supriya Kannery <address@hidden> wrote:
> On 05/20/2011 01:50 PM, Stefan Hajnoczi wrote:
>>
>> On Thu, May 19, 2011 at 10:38:03PM +0530, Supriya Kannery wrote:
>>>
>>> Monitor commands "hostcache_set" and "hostcache_get" added for dynamic
>>> host cache change and display of host cache setting respectively.
>>
>> A generic command for changing block device options would be nice,
>> althought I don't see other options where it makes sense to change them
>> at runtime.
>>
>> The alternative would be:
>>
>> block_set hostcache on
>>
>> "block_set", {"device": "ide1-cd0", "name": "hostcache", "enable": true}
>>
>> The hostcache_get information would be part of query-block output:
>>          {
>>             "device":"ide0-hd0",
>>             "locked":false,
>>             "removable":false,
>>             "inserted":{
>>                "ro":false,
>>                "drv":"qcow2",
>>                "encrypted":false,
>>                "file":"disks/test.img"
>>               "hostcache":true,
>>             },
>>             "type":"hd"
>>          },
>>
>> This approach is extensible if more options need to be exposed.
>
> Sure, I will resubmit this patchset, after making this feature more generic.
> Can you pls help finding atleast one or two options (other than hostcache)
> which can be changed dynamically. This will help me evaluate the generic
> approach.

Hang on, let's see if we can get agreement from Kevin and others
before taking this approach.  Like I said, I don't see other options
that should be changed at runtime.

>
>>> +        .params     = "device",
>>> +        .help       = "retrieve host cache settings for device",
>>
>> Please make it clear these operations affect block devices:
>> "for block device"
>
> ok
>
>>
>>> +    /*
>>> +     * A failed attempt to reopen the image file must lead to 'abort()'
>>> +     */
>>> +    if (ret != 0) {
>>> +        qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
>>> +        abort();
>>
>> The error is never reported on a QMP monitor because qerror_report()
>> simply stashes away the qerror.  The QMP client doesn't have a chance to
>> read the error before QEMU terminates.
>>
>
> Verified this from qemu command line and the error got displayed before
> aborting (when the image file was renamed while VM was running).

QMP takes a different code path from the human monitor (HMP) that you
used; see monitor_cur_is_qmp() and how it affects the qerror_report()
path.  CCing Luiz to see whether there is a strategy that we should
use here to ensure the error message does get delivered to the QMP
client before exiting.

Stefan