On Mon, Sep 30, 2024 at 16:53:55 +0200, Pavel Hrdina wrote:
> On Mon, Sep 30, 2024 at 03:29:34PM +0200, Peter Krempa wrote:
> > The new 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' migration
> > parameter allows users of migration to pass in a list of disks where
> > zero-detection (which avoids transferring the zeroed-blocks) should be
> > enabled for the migration connection. This comes at the cost of extra
> > CPU cycles needed to check each block if it's all-zero.
> >
> > This is useful for storage backends where information about the
> > allocation state of a block is not available and thus without this the
> > image would become fully allocated on the destination.
> >
> > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> > ---
> > include/libvirt/libvirt-domain.h | 13 ++++
> > src/qemu/qemu_driver.c | 20 ++++--
> > src/qemu/qemu_migration.c | 105 +++++++++++++++++++++++--------
> > src/qemu/qemu_migration.h | 4 ++
> > 4 files changed, 110 insertions(+), 32 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> > index 4266237abe..6d4cc69c5d 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -1240,6 +1240,19 @@ typedef enum {
> > */
> > # define VIR_MIGRATE_PARAM_MIGRATE_DISKS "migrate_disks"
> >
> > +/**
> > + * VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES:
> > + *
> > + * virDomainMigrate* params multiple field: The multiple values that list
> > + * the block devices for which zero detection (to avoid transferring zero blocks)
> > + * is to be enabled. This may increase CPU overhead of the migration. At the
> > + * moment this is only supported by the QEMU driver but not for the tunnelled
> > + * migration.
>
> We should note that it has to be subset of migrate_disks values.
I'd argue that it's logical that this can apply only if the disk is
being migrated.
>I also
> wonder if we should add a code that will error out if it's not the case,
> currently it would be silently ignored.
I'm firmly in the 'no' region. This can be used also if 'migrate_disks'
is not used so we'd have to cross check against the logic if a disk is
even being migrated.
As written above I'm okay declaring that we "did in fact enable
zero-detection for given migration" if the disk was not
migrated as all zeroes were properly 100% absolutely surely always
detected during that migration. Yes I'm being sarcastic. No I'll not add
this pointless checking code. I was borderline thinking not adding the
check whether the disk targets are valid, but that could be easily
extracted&reused.
