Re: [PATCH v5 5/8] hw/nvme: add migration blockers for non-supported cases

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3015 bytes --]

On Wed, Apr 08, 2026 at 08:32:38AM +0200, Klaus Jensen wrote:
> On Apr  7 20:39, Alexander Mikhalitsyn wrote:
> > Am Di., 7. Apr. 2026 um 17:34 Uhr schrieb Stefan Hajnoczi <stefanha@redhat.com>:
> > >
> > > On Tue, Mar 17, 2026 at 11:27:05AM +0100, Alexander Mikhalitsyn wrote:
> > > > +static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
> > > > +{
> > > > +    uint64_t unsupported_cap, cap = ldq_le_p(&n->bar.cap);
> > > > +    char blocker_features[BLOCKER_FEATURES_MAX_LEN] = "";
> > > > +    bool adm_cmd_security_checked = false;
> > > > +    bool cmd_io_mgmt_checked = false;
> > > > +    bool cmd_zone_checked = false;
> > > > +
> > > > +    /*
> > > > +     * Idea of this function is simple, we iterate over all Command Sets and
> > > > +     * for each supported command we provide a special handling logic to
> > > > +     * determine if we should block migration or not.
> > > > +     *
> > > > +     * For instance, we have NVME_ADM_CMD_NS_ATTACHMENT and it is always
> > > > +     * available to the guest, but if there is only 1 namespace, then it is
> > > > +     * safe to allow migration, but if there are more, then we need to block
> > > > +     * migration because we don't handle this in migration code yet.
> > >
> > 
> > Hi Stefan,
> > 
> > first of all thank you for looking into my patchset.
> > 
> > > What would be required to migrate multiple namespaces?
> > 
> > It should not be very complex, and starting from v4 I've added
> > VMStateDescription for nvme_ns, but I'm trying
> > to be extremely careful here, and keep patchset as small as possible
> > (and easy to review), because I it is my first
> > contribution to QEMU and I'm newbie with NVMe protocol. I wanted to
> > avoid any non-absolutely-required features
> > on the first approach and then add all extras after and on-demand.
> > 
> > My current understanding is that to support multiple namespaces we need:
> > - VMStateDescription for nvme_ns [ done ]
> > - add logic to handle (NvmeCtrl)->namespaces field and save info about
> > attached namespaces
> >   to some temporary field (use VMSTATE_WITH_TMP?)
> > - on restore we should iterate over namespaces attached and call
> > nvme_attach_ns(n, .)
> > 
> > Feels like this is it, but I can miss something. Please, let me know
> > if you believe that implementing this is absolutely
> > required and I'll look closer and add this in v6.
> > 
> 
> I do not think multi-ns support is a hard requirement. Multi namespace
> support is among the features that are mainly used for driver/host
> test and verification purposes.

Adding it incrementally is fine as long as it can be done in a
compatible way. Most of the time that is possible, although adding
something in a compatible way can be more complicated than implementing
it from the start.

In this case it sounds like the extra state can be added as a subsection
and it won't be too complicated.

Thanks,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]