| Message ID | 1406271172-1192-6-git-send-email-arei.gonglei@huawei.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > 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": "/disk@0"}} > <- { "return": {} } > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > Signed-off-by: Chenliang <chenliang88@huawei.com> > --- > 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?
Hi, > Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command > > On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gonglei@huawei.com wrote: > > From: Gonglei <arei.gonglei@huawei.com> > > > > 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": "/disk@0"}} > > <- { "return": {} } > > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > > Signed-off-by: Chenliang <chenliang88@huawei.com> > > --- > > 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. Best regards, -Gonglei
"Gonglei (Arei)" <arei.gonglei@huawei.com> 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, arei.gonglei@huawei.com wrote: >> > From: Gonglei <arei.gonglei@huawei.com> >> > >> > 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": "/disk@0"}} >> > <- { "return": {} } >> > >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> >> > --- >> > 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. 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. 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. You either set bootindexA or bootindexB. No need to fuzz around with suffixes. Or am I missing something?
Hi, > >> Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command > >> > >> On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gonglei@huawei.com wrote: > >> > From: Gonglei <arei.gonglei@huawei.com> > >> > > >> > 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": "/disk@0"}} > >> > <- { "return": {} } > >> > > >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > >> > Signed-off-by: Chenliang <chenliang88@huawei.com> > >> > --- > >> > 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? Best regards, -Gonglei
"Gonglei (Arei)" <arei.gonglei@huawei.com> 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, arei.gonglei@huawei.com wrote: >> >> > From: Gonglei <arei.gonglei@huawei.com> >> >> > >> >> > 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": "/disk@0"}} >> >> > <- { "return": {} } >> >> > >> >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> >> >> > --- >> >> > 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.
Hi, Markus > >> >> Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex > command > >> >> > >> >> On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gonglei@huawei.com > wrote: > >> >> > From: Gonglei <arei.gonglei@huawei.com> > >> >> > > >> >> > 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": "/disk@0"}} > >> >> > <- { "return": {} } > >> >> > > >> >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > >> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> > >> >> > --- > >> >> > 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. Thanks for your explaining. TBH I haven't used "qom-set" before, so I don't know what your mean (like Eduardo, sorry again). But now, I have a look at the implement of "qom-set" command, and I find the command is just change a device's property value, do not have any other logic. For my case, we really change value of the global fw_boot_order list, which the device's bootindex property worked for actually. Only in this way, we can attach our original target. Best regards, -Gonglei
"Gonglei (Arei)" <arei.gonglei@huawei.com> writes: > Hi, Markus > >> >> >> Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex >> command >> >> >> >> >> >> On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gonglei@huawei.com >> wrote: >> >> >> > From: Gonglei <arei.gonglei@huawei.com> >> >> >> > >> >> >> > 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": "/disk@0"}} >> >> >> > <- { "return": {} } >> >> >> > >> >> >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> >> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> >> >> >> > --- >> >> >> > 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. > > Thanks for your explaining. TBH I haven't used "qom-set" before, so I Few people have :) > don't know what your mean (like Eduardo, sorry again). > > But now, I have a look at the implement of "qom-set" command, and > I find the command is just change a device's property value, do not have > any other logic. For my case, we really change value of the global > fw_boot_order list, > which the device's bootindex property worked for actually. Only in this way, > we can attach our original target. Why can't we delay the logic to the next reboot? Let me explain. Current code starts with empty fw_boot_order, then lets device realize add to it. Unplugging a device leaves a dangling DeviceState pointer in the list (I think). Could we instead build, use and free the list during reboot? That way, qom-set of bootindex affects the next reboot without additional logic.
Hi, Markus > > > >> >> >> Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex > >> command > >> >> >> > >> >> >> On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gonglei@huawei.com > >> wrote: > >> >> >> > From: Gonglei <arei.gonglei@huawei.com> > >> >> >> > > >> >> >> > 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": "/disk@0"}} > >> >> >> > <- { "return": {} } > >> >> >> > > >> >> >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > >> >> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> > >> >> >> > --- > >> >> >> > 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. > > > > Thanks for your explaining. TBH I haven't used "qom-set" before, so I > > Few people have :) > > > don't know what your mean (like Eduardo, sorry again). > > > > But now, I have a look at the implement of "qom-set" command, and > > I find the command is just change a device's property value, do not have > > any other logic. For my case, we really change value of the global > > fw_boot_order list, > > which the device's bootindex property worked for actually. Only in this way, > > we can attach our original target. > > Why can't we delay the logic to the next reboot? Let me explain. > > Current code starts with empty fw_boot_order, then lets device realize > add to it. Unplugging a device leaves a dangling DeviceState pointer in > the list (I think). > Yes. > Could we instead build, use and free the list during reboot? That way, > qom-set of bootindex affects the next reboot without additional logic. Yes, we can do this, but it will be too complicated. Firstly the device realize will not reattach during reboot. So, we should check all the device of the global list during reboot, but the DeviceState haven't bootindex property, we should cast it into idiographic device for getting the changed bootindex, such as " VirtIONet *n = VIRTIO_NET(dev)" ( this will be a trouble for different devices), then we can change fw_boot_order list's bootindex using new bootindex, then reorder all bootindexes in list. Am I wrong? So, I don't think it is a good idea. Best regards, -Gonglei
"Gonglei (Arei)" <arei.gonglei@huawei.com> writes: > Hi, Markus >> > >> >> >> >> Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex >> >> command >> >> >> >> >> >> >> >> On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gonglei@huawei.com >> >> wrote: >> >> >> >> > From: Gonglei <arei.gonglei@huawei.com> >> >> >> >> > >> >> >> >> > 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": "/disk@0"}} >> >> >> >> > <- { "return": {} } >> >> >> >> > >> >> >> >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> >> >> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> >> >> >> >> > --- >> >> >> >> > 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. >> > >> > Thanks for your explaining. TBH I haven't used "qom-set" before, so I >> >> Few people have :) >> >> > don't know what your mean (like Eduardo, sorry again). >> > >> > But now, I have a look at the implement of "qom-set" command, and >> > I find the command is just change a device's property value, do not have >> > any other logic. For my case, we really change value of the global >> > fw_boot_order list, >> > which the device's bootindex property worked for actually. Only in this way, >> > we can attach our original target. >> >> Why can't we delay the logic to the next reboot? Let me explain. >> >> Current code starts with empty fw_boot_order, then lets device realize >> add to it. Unplugging a device leaves a dangling DeviceState pointer in >> the list (I think). >> > Yes. > >> Could we instead build, use and free the list during reboot? That way, >> qom-set of bootindex affects the next reboot without additional logic. > > Yes, we can do this, but it will be too complicated. Firstly the device realize > will not reattach during reboot. So, we should check all the device of > the global list during reboot, > but the DeviceState haven't bootindex property, we should cast it into > idiographic > device for getting the changed bootindex, such as " VirtIONet *n = > VIRTIO_NET(dev)" ( this will be a trouble > for different devices), then we can change fw_boot_order list's > bootindex using new bootindex, > then reorder all bootindexes in list. Am I wrong? > > So, I don't think it is a good idea. Moving add_boot_device_path() from realize to reset could do the trick. Anyway, delaying the logic is not necessary for use of qom-set. Ordinary qdev properties use common setter functions. QOM is more general: it lets you define your own setter. For example, device_initfn() defines property "realized" like this: object_property_add_bool(obj, "realized", device_get_realized, device_set_realized, NULL);
On Mon, Aug 04, 2014 at 12:00:59PM +0200, Markus Armbruster wrote: > "Gonglei (Arei)" <arei.gonglei@huawei.com> 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, arei.gonglei@huawei.com wrote: > >> >> > From: Gonglei <arei.gonglei@huawei.com> > >> >> > > >> >> > 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": "/disk@0"}} > >> >> > <- { "return": {} } > >> >> > > >> >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > >> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> > >> >> > --- > >> >> > 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. The "path" argument may be a "partial path". From qapi-schema.json: Partial paths look like relative filenames. They do not begin with a prefix. The matching rules for partial paths are subtle but designed to make specifying objects easy. At each level of the composition tree, the partial path is matched as an absolute path. The first match is not returned. At least two matches are searched for. A successful result is only returned if only one match is found. If more than one match is found, a flag is return to indicate that the match was ambiguous. I haven't tested this. Is the qdev ID visible in device paths somewhere, allowing it to be used for partial path matching?
Hi, > >> >> >> >> Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex > >> >> command > >> >> >> >> > >> >> >> >> On Fri, Jul 25, 2014 at 02:52:50PM +0800, > arei.gonglei@huawei.com > >> >> wrote: > >> >> >> >> > From: Gonglei <arei.gonglei@huawei.com> > >> >> >> >> > > >> >> >> >> > 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": "/disk@0"}} > >> >> >> >> > <- { "return": {} } > >> >> >> >> > > >> >> >> >> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > >> >> >> >> > Signed-off-by: Chenliang <chenliang88@huawei.com> > >> >> >> >> > --- > >> >> >> >> > 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. > >> > > >> > Thanks for your explaining. TBH I haven't used "qom-set" before, so I > >> > >> Few people have :) > >> > >> > don't know what your mean (like Eduardo, sorry again). > >> > > >> > But now, I have a look at the implement of "qom-set" command, and > >> > I find the command is just change a device's property value, do not have > >> > any other logic. For my case, we really change value of the global > >> > fw_boot_order list, > >> > which the device's bootindex property worked for actually. Only in this way, > >> > we can attach our original target. > >> > >> Why can't we delay the logic to the next reboot? Let me explain. > >> > >> Current code starts with empty fw_boot_order, then lets device realize > >> add to it. Unplugging a device leaves a dangling DeviceState pointer in > >> the list (I think). > >> > > Yes. > > > >> Could we instead build, use and free the list during reboot? That way, > >> qom-set of bootindex affects the next reboot without additional logic. > > > > Yes, we can do this, but it will be too complicated. Firstly the device realize > > will not reattach during reboot. So, we should check all the device of > > the global list during reboot, > > but the DeviceState haven't bootindex property, we should cast it into > > idiographic > > device for getting the changed bootindex, such as " VirtIONet *n = > > VIRTIO_NET(dev)" ( this will be a trouble > > for different devices), then we can change fw_boot_order list's > > bootindex using new bootindex, > > then reorder all bootindexes in list. Am I wrong? > > > > So, I don't think it is a good idea. > > Moving add_boot_device_path() from realize to reset could do the trick. > Hmm... but not every device has reset function. This changing will be a big surgery IMO. > Anyway, delaying the logic is not necessary for use of qom-set. > Ordinary qdev properties use common setter functions. QOM is more > general: it lets you define your own setter. For example, > device_initfn() defines property "realized" like this: > > object_property_add_bool(obj, "realized", > device_get_realized, device_set_realized, > NULL); Agreed. Best regards, -Gonglei
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'} } + +## # @DumpGuestMemoryFormat: # # An enumeration of guest-memory-dump's format. diff --git a/qmp-commands.hx b/qmp-commands.hx index 4be4765..2c89a97 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -330,6 +330,30 @@ Example: <- { "return": {} } EQMP + { + .name = "set-bootindex", + .args_type = "id:s,bootindex:l,suffix:s?", + .mhandler.cmd_new = qmp_marshal_input_set_bootindex, + }, + +SQMP +set-bootindex +-------------------- + +Set bootindex of a device + +Arguments: + +- "id": the device's ID (json-string) +- "bootindex": the device's bootindex (json-int) +- "suffix": the device's suffix in global boot list (json-string, optional) + +Example: + +-> { "execute": "set-bootindex", "arguments": { "id": "ide0-0-1", "bootindex": 1, "suffix": "/disk@0"}} +<- { "return": {} } + +EQMP { .name = "send-key", diff --git a/qmp.c b/qmp.c index 0d2553a..f2c3c14 100644 --- a/qmp.c +++ b/qmp.c @@ -684,6 +684,23 @@ void qmp_object_del(const char *id, Error **errp) object_unparent(obj); } +void qmp_set_bootindex(const char *id, int64_t bootindex, + bool has_suffix, const char *suffix, Error **errp) +{ + DeviceState *dev; + + dev = qdev_find_recursive(sysbus_get_default(), id); + if (NULL == dev) { + error_set(errp, QERR_DEVICE_NOT_FOUND, id); + return; + } + if (has_suffix) { + modify_boot_device_path(bootindex, dev, suffix); + } else { + modify_boot_device_path(bootindex, dev, NULL); + } +} + MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp) { MemoryDeviceInfoList *head = NULL;