All of lore.kernel.org
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	"open list:nvme" <qemu-block@nongnu.org>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Joelle van Dyne <j@getutm.app>, Keith Busch <kbusch@kernel.org>
Subject: Re: [PATCH] nvme: expose 'bootindex' property
Date: Mon, 22 Mar 2021 13:37:18 +0100	[thread overview]
Message-ID: <YFiPfnX8is7tCS16@apples.localdomain> (raw)
In-Reply-To: <a7030c4b-943d-c88d-655d-dfd6398d68d5@redhat.com>

[-- 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 --]

  reply	other threads:[~2021-03-22 12:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2021-03-22 13:10     ` Philippe Mathieu-Daudé
2021-03-22 13:19       ` Klaus Jensen
2021-03-30 18:10 ` Klaus Jensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YFiPfnX8is7tCS16@apples.localdomain \
    --to=its@irrelevant.dk \
    --cc=j@getutm.app \
    --cc=kbusch@kernel.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.