On Wed, Feb 22, 2012 at 12:54:35 -0700, Eric Blake wrote:
> On 02/22/2012 07:51 AM, Jiri Denemark wrote:
> > Migrating domains with disks using cache != none is unsafe unless the
> > disk images are stored on coherent clustered filesystem. Thus we forbid
> > migrating such domains unless VIR_MIGRATE_UNSAFE flags is used.
> > ---
> > Notes:
> > Version 2:
> > - use virStorageFileIsClusterFS
> >
> > src/qemu/qemu_driver.c | 3 ++-
> > src/qemu/qemu_migration.c | 39 +++++++++++++++++++++++++++++++++++----
> > src/qemu/qemu_migration.h | 6 ++++--
> > 3 files changed, 41 insertions(+), 7 deletions(-)
> >
>
> >
> > +static bool
> > +qemuMigrationIsSafe(virDomainDefPtr def)
> > +{
> > + int i;
> > +
> > + for (i = 0 ; i < def->ndisks ; i++) {
> > + virDomainDiskDefPtr disk = def->disks[i];
> > +
> > + /* shared && !readonly implies cache=none */
> > + if (disk->src &&
> > + disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE &&
> > + (disk->cachemode || !disk->shared || disk->readonly) &&
> > + virStorageFileIsClusterFS(disk->src) == 1) {
>
> Other than Dan's comment about the logic bug here, ACK.
Actually, I rewrote this as
+ if (disk->src &&
+ disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE &&
+ (disk->cachemode || !disk->shared || disk->readonly)) {
+ int cfs;
+ if ((cfs = virStorageFileIsClusterFS(disk->src)) == 1)
+ continue;
+ else if (cfs < 0)
+ return false;
+
+ qemuReportError(VIR_ERR_MIGRATE_UNSAFE, "%s",
+ _("Migration may lead to data corruption if disks"
+ " use cache != none"));
+ return false;
+ }
to avoid overwriting errors returned by virStorageFileIsClusterFS().
> Since the check for safety is only on the source, and the destination
> doesn't care, is there a way to add a driver feature flag, and add logic
> to libvirt.c to mask the VIR_MIGRATE_UNSAFE flag from the destination if
> it does not support the feature, similar to how we handled
> VIR_MIGRATE_CHANGE_PROTECTION via the
> VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION feature, as another example of
> a source-only flag? Of course, this would be a followup patch, if we
> decide it is worth allowing an unsafe migration from 1.9.11 back to
> 1.9.10 (the upgrade migration from 1.9.10 to 1.9.11 will be unsafe
> automatically, because we weren't checking in 1.9.10).
Yeah, it would be possible to do it, however as we already added at least new
qemu capability (system_wakeup), domains running on new enough qemu will not
be migratable to 0.9.10 anyway so I guess it's just not worth it :-)
And I pushed this series, thanks for the reviews.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list