[PATCH] qemuDomainChangeDiskLive: Modify 'startupPolicy' before changing source

[Vojtech Juranek]

On Monday, 13 September 2021 13:30:45 CEST Nir Soffer wrote:
> On Mon, Sep 13, 2021 at 11:04 AM Peter Krempa <pkrempa at redhat.com> wrote:
> > On Fri, Sep 10, 2021 at 22:04:01 +0300, Nir Soffer wrote:
> > > On Fri, Sep 10, 2021 at 4:35 PM Peter Krempa <pkrempa at redhat.com> wrote:
> > > > We don't support all startup policies with all source types so to
> > > > correctly allow switching from a 'file' based cdrom with 'optional'
> > > > startup policy to a 'block' based one which doesn't support optional
> > > > we
> > > > must update the startup policy field first. Obviously we need to have
> > > > fallback if the update fails.
> > > 
> > > Why is there a difference between file and block for startup policy?
> > 
> > I'm not sure about the historic reasons for why 'block' was not
> > considered when this was implemented originally.
> > 
> > > Is this documented?
> > 
> > (citation from 'formatdomain.rst' the source for
> > https://www.libvirt.org/formatdomain.html )> 
> >    For a "file" or "volume" disk type which represents a cdrom or floppy
> >    (the
> >    ``device`` attribute), it is possible to define policy what to do with
> >    the
> >    disk if the source file is not accessible. (NB, ``startupPolicy`` is
> >    not
> >    valid for "volume" disk unless the specified storage volume is of
> >    "file"
> >    type). This is done by the ``startupPolicy`` attribute ( :since:`since
> >    0.9.7` ), accepting these values:
> >    
> >    =========
> >    =====================================================================
> >    mandatory fail if missing for any reason (the default)
> >    requisite fail if missing on boot up, drop if missing on
> >    migrate/restore/revert optional  drop if missing at any start attempt
> >    =========
> >    =====================================================================>    
> >    :since:`Since 1.1.2` the ``startupPolicy`` is extended to support hard
> >    :disks
> >    
> >    besides cdrom and floppy. On guest cold bootup, if a certain disk is
> >    not
> >    accessible or its disk chain is broken, with startupPolicy 'optional'
> >    the
> >    guest will drop this disk. This feature doesn't support migration
> >    currently.> 
> > As you can notice only "file" and "volume" (pointing to a file) are
> > allowed.
> 
> Looks like we missed this detail when adding support for cdrom on
> block storage.
> 
> > > Do we have a way to switch from file to block with current libvirt
> > > (centos 8 stream, rhel 8.4) without this patch, or do we need to wait
> > > until this fix is available? (rhel 8.5?)
> > 
> > You can issue two separate update calls. One updating the startup policy
> > first and then the second one updating the source.
> 
> If the second call fails, this leave us with half of the change.
> 
> > On another note, I don't quite understand though why oVirt actually even
> > uses startup policy in the first place. Since oVirt is already doing a
> > pretty complicated storage management, checking whether the image file
> > exists is a trivial operation.
> 
> In oVirt we know that the image exists since we prepare it before
> starting the VM
> (e.g. activate logical volume), and we will fail the operation if the
> disk does not
> exists, so I think this attribute is not needed for our use case and
> it should be
> removed.

+1

> > Similarly, when migrating you can use the destination XML (which can be
> > optionally used with the migration API) allows you to update storage
> > source and even provide empty storage for a cdrom. This can be used to
> > update paths to storage too. This allows superior flexibility when
> > compared to startup policy.
> 
> Even if we remove startupPolicy in new oVirt version, we have to
> support migration
> from an older version using startupPolicy in the xml.

What is the problem here? The issue happens only when you try to change the 
CD, not during the migration, so there shouldn't be any problem (I also tried 
it now and I can migrate VM with block-based CD without any issue). The issue 
is there only when VM is started with empty CD or file-based CD, as in these 
cases we added startupPolicy into the XML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210913/5cb262d2/attachment-0001.sig>