Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command

[Markus Armbruster]

"Gonglei (Arei)" <address@hidden> writes:

> Hi,
>
>> >> Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
>> >>
>> >> On Fri, Jul 25, 2014 at 02:52:50PM +0800, address@hidden wrote:
>> >> > From: Gonglei <address@hidden>
>> >> >
>> >> > Adds "set-bootindex id=xx,bootindex=xx,suffix=xx" QMP command.
>> >> >
>> >> > Example QMP command:
>> >> > -> { "execute": "set-bootindex", "arguments": { "id": "ide0-0-1",
>> >> > "bootindex":
>> >> 1, "suffix": "/address@hidden"}}
>> >> > <- { "return": {} }
>> >> >
>> >> > Signed-off-by: Gonglei <address@hidden>
>> >> > Signed-off-by: Chenliang <address@hidden>
>> >> > ---
>> >> >  qapi-schema.json | 16 ++++++++++++++++
>> >> >  qmp-commands.hx  | 24 ++++++++++++++++++++++++
>> >> >  qmp.c            | 17 +++++++++++++++++
>> >> >  3 files changed, 57 insertions(+)
>> >> >
>> >> > diff --git a/qapi-schema.json b/qapi-schema.json
>> >> > index b11aad2..a9ef0be 100644
>> >> > --- a/qapi-schema.json
>> >> > +++ b/qapi-schema.json
>> >> > @@ -1704,6 +1704,22 @@
>> >> >  { 'command': 'device_del', 'data': {'id': 'str'} }
>> >> >
>> >> >  ##
>> >> > +# @set-bootindex:
>> >> > +#
>> >> > +# set bootindex of a devcie
>> >> > +#
>> >> > +# @id: the name of the device
>> >> > +# @bootindex: the bootindex of the device
>> >> > +# @suffix: #optional a suffix of the device
>> >> > +#
>> >> > +# Returns: Nothing on success
>> >> > +#          If @id is not a valid device, DeviceNotFound
>> >> > +#
>> >> > +# Since: 2.2
>> >> > +##
>> >> > +{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex':
>> >> > int', '*suffix':
>> >> 'str'} }
>> >> > +
>> >> > +##
>> >>
>> >> I wonder if we could simply use qom-set for that. How many devices
>> >> actually support having multiple bootindex entries with different
>> >> suffixes?
>> >>
>> > AFAICT, the floppy device support two bootindex with different suffixes.
>> 
>> Block device models commonly a single block device.  By convention,
>> property "drive" is the backend, and property "bootindex" the bootindex,
>> if the device model supports that.
>> 
>> The device ID suffices to identify a block device for such device
>> models.
>> 
>> The floppy device model is an exception.  It folds multiple real-world
>> objects into one: the controller and the actual drives.  Have a look at
>> -device isa-fdc,help:
>> 
>>     isa-fdc.iobase=uint32
>>     isa-fdc.irq=uint32
>>     isa-fdc.dma=uint32
>>     isa-fdc.driveA=drive
>>     isa-fdc.driveB=drive
>>     isa-fdc.bootindexA=int32
>>     isa-fdc.bootindexB=int32
>>     isa-fdc.check_media_rate=on/off
>> 
>> The properties ending with 'A' or 'B' apply to the first and the second
>> drive, the others to the controller.
>> 
>> Unfortunately, this means the device ID by itself doesn't identify the
>> floppy device.
>> 
> Yes.
>
>> Arguably, this is lousy modeling --- we really should model separate
>> real-world objects as separate objects.  But making floppies pretty (or
>> even sane) isn't anyone's priority nowadays.
>> 
> Hmm... Agreed.
>
>> Since qom-set works on *properties*, I can't see why it couldn't be used
>> for changing bootindexes.  There is no ambiguity even with floppy.
>
> Sorry? 
>
>> You either set bootindexA or bootindexB.  No need to fuzz around with
>> suffixes.  Or am I missing something?
>
> Your mean that we just need to think about change one bootindex? Either 
> set bootindexA or bootindexB, do not need pass suffix for identifying?

Eduardo suggested to think about using qom-set to update the bootindex.

Could look like

    { "execute": "qom-set",
      "arguments": { "path": "/machine/unattached/device[15]",
                     "property": "bootindexA", "value": 1 } }

Demonstrates an existing, well-defined way to identify the bootindex to
change.  Do we really want to invent another one, based on suffix?

The value of "path" is the QOM path.  I can't remember offhand how to go
from qdev ID to QOM path.  Onboard devices like isa-fdc don't have one
anyway.

I also don't remember whether there's a nicer QOM path than this one.