* [PATCH] nvme: expose 'bootindex' property
@ 2021-03-22 8:24 Joelle van Dyne
2021-03-22 9:58 ` Philippe Mathieu-Daudé
2021-03-30 18:10 ` Klaus Jensen
0 siblings, 2 replies; 6+ messages in thread
From: Joelle van Dyne @ 2021-03-22 8:24 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, open list:nvme, Max Reitz, Klaus Jensen,
Joelle van Dyne, Keith Busch
The check for `n->namespace.blkconf.blk` always fails because
this is in the initialization function.
Signed-off-by: Joelle van Dyne <j@getutm.app>
---
hw/block/nvme.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 6842b01ab5..42605fc55d 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -6330,11 +6330,9 @@ static void nvme_instance_init(Object *obj)
{
NvmeCtrl *n = NVME(obj);
- if (n->namespace.blkconf.blk) {
- device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
- "bootindex", "/namespace@1,0",
- DEVICE(obj));
- }
+ device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
+ "bootindex", "/namespace@1,0",
+ DEVICE(obj));
object_property_add(obj, "smart_critical_warning", "uint8",
nvme_get_smart_warning,
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: expose 'bootindex' property
2021-03-22 8:24 [PATCH] nvme: expose 'bootindex' property Joelle van Dyne
@ 2021-03-22 9:58 ` Philippe Mathieu-Daudé
2021-03-22 12:37 ` Klaus Jensen
2021-03-30 18:10 ` Klaus Jensen
1 sibling, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 9:58 UTC (permalink / raw)
To: Joelle van Dyne, qemu-devel
Cc: Kevin Wolf, Klaus Jensen, Keith Busch, open list:nvme, Max Reitz
On 3/22/21 9:24 AM, Joelle van Dyne wrote:
> The check for `n->namespace.blkconf.blk` always fails because
> this is in the initialization function.
This usually mean the code depends to some state only available
during the QOM 'realization' step, so this code should be in
nvme_realize(). Maybe in this case we don't need it there and
can add the property regardless a block drive is provided, I
haven't checked.
>
> Signed-off-by: Joelle van Dyne <j@getutm.app>
> ---
> hw/block/nvme.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 6842b01ab5..42605fc55d 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -6330,11 +6330,9 @@ static void nvme_instance_init(Object *obj)
> {
> NvmeCtrl *n = NVME(obj);
>
> - if (n->namespace.blkconf.blk) {
> - device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
> - "bootindex", "/namespace@1,0",
> - DEVICE(obj));
> - }
> + device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
> + "bootindex", "/namespace@1,0",
> + DEVICE(obj));
>
> object_property_add(obj, "smart_critical_warning", "uint8",
> nvme_get_smart_warning,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: expose 'bootindex' property
2021-03-22 9:58 ` Philippe Mathieu-Daudé
@ 2021-03-22 12:37 ` Klaus Jensen
2021-03-22 13:10 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Klaus Jensen @ 2021-03-22 12:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, open list:nvme, qemu-devel, Max Reitz,
Joelle van Dyne, Keith Busch
[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]
On Mar 22 10:58, Philippe Mathieu-Daudé wrote:
> On 3/22/21 9:24 AM, Joelle van Dyne wrote:
> > The check for `n->namespace.blkconf.blk` always fails because
> > this is in the initialization function.
>
> This usually mean the code depends to some state only available
> during the QOM 'realization' step, so this code should be in
> nvme_realize(). Maybe in this case we don't need it there and
> can add the property regardless a block drive is provided, I
> haven't checked.
>
If we defer to realization, it won't be available as a parameter on the
command line, but as far as I can test, adding it unconditionally
doesn't break anything when there is no drive attached to the controller
device.
> >
> > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > ---
> > hw/block/nvme.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 6842b01ab5..42605fc55d 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -6330,11 +6330,9 @@ static void nvme_instance_init(Object *obj)
> > {
> > NvmeCtrl *n = NVME(obj);
> >
> > - if (n->namespace.blkconf.blk) {
> > - device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
> > - "bootindex", "/namespace@1,0",
> > - DEVICE(obj));
> > - }
> > + device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
> > + "bootindex", "/namespace@1,0",
> > + DEVICE(obj));
> >
> > object_property_add(obj, "smart_critical_warning", "uint8",
> > nvme_get_smart_warning,
> >
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: expose 'bootindex' property
2021-03-22 12:37 ` Klaus Jensen
@ 2021-03-22 13:10 ` Philippe Mathieu-Daudé
2021-03-22 13:19 ` Klaus Jensen
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 13:10 UTC (permalink / raw)
To: Klaus Jensen
Cc: Kevin Wolf, open list:nvme, qemu-devel, Max Reitz,
Joelle van Dyne, Keith Busch
On 3/22/21 1:37 PM, Klaus Jensen wrote:
> On Mar 22 10:58, Philippe Mathieu-Daudé wrote:
>> On 3/22/21 9:24 AM, Joelle van Dyne wrote:
>>> The check for `n->namespace.blkconf.blk` always fails because
>>> this is in the initialization function.
>>
>> This usually mean the code depends to some state only available
>> during the QOM 'realization' step, so this code should be in
>> nvme_realize(). Maybe in this case we don't need it there and
>> can add the property regardless a block drive is provided, I
>> haven't checked.
>>
>
> If we defer to realization, it won't be available as a parameter on the
> command line, but as far as I can test, adding it unconditionally
> doesn't break anything when there is no drive attached to the controller
> device.
Patch is good then :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: expose 'bootindex' property
2021-03-22 13:10 ` Philippe Mathieu-Daudé
@ 2021-03-22 13:19 ` Klaus Jensen
0 siblings, 0 replies; 6+ messages in thread
From: Klaus Jensen @ 2021-03-22 13:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, open list:nvme, qemu-devel, Max Reitz,
Joelle van Dyne, Keith Busch
[-- Attachment #1: Type: text/plain, Size: 960 bytes --]
On Mar 22 14:10, Philippe Mathieu-Daudé wrote:
> On 3/22/21 1:37 PM, Klaus Jensen wrote:
> > On Mar 22 10:58, Philippe Mathieu-Daudé wrote:
> >> On 3/22/21 9:24 AM, Joelle van Dyne wrote:
> >>> The check for `n->namespace.blkconf.blk` always fails because
> >>> this is in the initialization function.
> >>
> >> This usually mean the code depends to some state only available
> >> during the QOM 'realization' step, so this code should be in
> >> nvme_realize(). Maybe in this case we don't need it there and
> >> can add the property regardless a block drive is provided, I
> >> haven't checked.
> >>
> >
> > If we defer to realization, it won't be available as a parameter on the
> > command line, but as far as I can test, adding it unconditionally
> > doesn't break anything when there is no drive attached to the controller
> > device.
>
> Patch is good then :)
>
Agreed :)
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: expose 'bootindex' property
2021-03-22 8:24 [PATCH] nvme: expose 'bootindex' property Joelle van Dyne
2021-03-22 9:58 ` Philippe Mathieu-Daudé
@ 2021-03-30 18:10 ` Klaus Jensen
1 sibling, 0 replies; 6+ messages in thread
From: Klaus Jensen @ 2021-03-30 18:10 UTC (permalink / raw)
To: Joelle van Dyne
Cc: Keith Busch, Kevin Wolf, qemu-devel, open list:nvme, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]
On Mar 22 01:24, Joelle van Dyne wrote:
> The check for `n->namespace.blkconf.blk` always fails because
> this is in the initialization function.
>
> Signed-off-by: Joelle van Dyne <j@getutm.app>
> ---
> hw/block/nvme.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 6842b01ab5..42605fc55d 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -6330,11 +6330,9 @@ static void nvme_instance_init(Object *obj)
> {
> NvmeCtrl *n = NVME(obj);
>
> - if (n->namespace.blkconf.blk) {
> - device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
> - "bootindex", "/namespace@1,0",
> - DEVICE(obj));
> - }
> + device_add_bootindex_property(obj, &n->namespace.blkconf.bootindex,
> + "bootindex", "/namespace@1,0",
> + DEVICE(obj));
>
> object_property_add(obj, "smart_critical_warning", "uint8",
> nvme_get_smart_warning,
> --
> 2.28.0
>
Applied on nvme-fixes, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-30 18:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 8:24 [PATCH] nvme: expose 'bootindex' property Joelle van Dyne
2021-03-22 9:58 ` Philippe Mathieu-Daudé
2021-03-22 12:37 ` Klaus Jensen
2021-03-22 13:10 ` Philippe Mathieu-Daudé
2021-03-22 13:19 ` Klaus Jensen
2021-03-30 18:10 ` Klaus Jensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.