Re: [libvirt] [PATCH] qemu: fix migration with local and VIR_STORAGE_TYPE_NETWORK disks
On 02/08/2018 03:07 AM, Daniel P. Berrangé wrote:
On Wed, Feb 07, 2018 at 01:11:33PM -0600, Chris Friesen wrote:
> Are you okay with the other change?
That part of the code was intended to be funtionally identical to what
QEMU's previous built-in storage migration code would do. I don't want
to see the semantics of that change, because it makes libvirt behaviour
vary depending on which QEMU version you are using.
If that logic is not right for a particular usage scenario, applications
are expected to provide the "migrate_disks" parameter.
My coworker has pointed out another related issue.
In tools/virsh-domain.c, doMigrate(), if we specify "migrate-disks" with an
empty list, the behaviour is the same as if it is not specified at all. That
is, the fact that it was specified but empty is lost.
> Our original problem scenario was where the root disk is rbd and
there is a
> read-only ISO config-drive, and "nmigrate_disks" is zero. What we see in
> this case is that qemuMigrateDisk() returns "true" for the rbd disk, which
> then causes qemuMigrationPrecreateStorage() to fail with "pre-creation of
> storage targets for incremental storage migration is not supported".
So you want zero disks migrated. Simply don't ask for storage migration in
the first place if you don't have any disks to migrate.
In this case yes, but now we're talking about duplicating the libvirt logic
around which disks to migrate in the code that calls libvirt.
There is a comment in the OpenStack nova code that looks like this:
# Due to a quirk in the libvirt python bindings,
# VIR_MIGRATE_NON_SHARED_INC with an empty migrate_disks is
# interpreted as "block migrate all writable disks" rather than
# "don't block migrate any disks". This includes attached
# volumes, which will potentially corrupt data on those
# volumes. Consequently we need to explicitly unset
# VIR_MIGRATE_NON_SHARED_INC if there are no disks to be block
# migrated.
It sounds like it's not just a quirk, but rather design intent?
Given your comment above about "I don't want to see the semantics of that
change", it sounds like you're suggesting:
1) If there are any non-shared non-readonly network drives then the user can't
rely on the default behaviour of VIR_MIGRATE_NON_SHARED_INC to do the right
thing and therefore must explicitly specify the list of drives to migrate
2) If there are no drives to migrate, then it is not valid to specify
VIR_MIGRATE_NON_SHARED_INC with an empty "migrate_disks", but instead the caller
should ensure that VIR_MIGRATE_NON_SHARED_INC is not set.
Is that a fair summation? If so, I'd suggest that this is non-intuitive from a
user's perspective and at a minimum should be more explicitly documented.
Chris