On Wed, Jun 09, 2010 at 08:33:49PM -0600, Eric Blake wrote: > Followup to https://bugzilla.redhat.com/show_bug.cgi?id=599091, > commit 20206a4b, to reduce disk waste in padding. > > * src/qemu/qemu_monitor.h (QEMU_MONITOR_MIGRATE_TO_FILE_BS): Drop > back to 512. > (QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE): New macro. > * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Update comment. > * src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFile): Use > two invocations of dd to output non-aligned large blocks. > * src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFile): > Likewise. > --- > > This should result in much less wasted space (but I don't have a good > environment set up for testing migration at the moment; Laine, would > you mind testing this patch, since you wrote the last one?). > > Also, should we bump QEMU_MONITOR_MIGRATE_TO_FILE_BS to 4k, for the > benefit of newer disks that have 4k blocks? > > src/qemu/qemu_driver.c | 7 ------- > src/qemu/qemu_monitor.h | 12 ++++++++---- > src/qemu/qemu_monitor_json.c | 13 +++++++++---- > src/qemu/qemu_monitor_text.c | 13 +++++++++---- > 4 files changed, 26 insertions(+), 19 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index bc8dcfa..4edf5ec 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -5000,13 +5000,6 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path, > * we need to ensure there's a 512 byte boundary. Unfortunately > * we don't have an explicit offset in the header, so we fake > * it by padding the XML string with NULLs. > - * > - * XXX: This means there will be (QEMU_MONITOR_MIGRATE_TO_FILE_BS > - * - strlen(xml)) bytes of wastage in each file. > - * Unfortunately, a large BS is needed for reasonable > - * performance. It would be nice to find a replacement for dd > - * that could specify the start offset in bytes rather than > - * blocks, to eliminate this waste. > */ > if (offset % QEMU_MONITOR_MIGRATE_TO_FILE_BS) { > unsigned long long pad = > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index e03b4df..8391ebd 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -1,7 +1,7 @@ > /* > * qemu_monitor.h: interaction with QEMU monitor console > * > - * Copyright (C) 2006-2009 Red Hat, Inc. > + * Copyright (C) 2006-2010 Red Hat, Inc. > * Copyright (C) 2006 Daniel P. Berrange > * > * This library is free software; you can redistribute it and/or > @@ -261,10 +261,14 @@ int qemuMonitorMigrateToCommand(qemuMonitorPtr mon, > unsigned int background, > const char * const *argv); > > -/* In general, a larger BS means better domain save performance, > - * at the expense of a larger resulting file - see qemu_driver.c > +/* In general, BS is the smallest fundamental block size we can use to > + * access a block device; everything must be aligned to a multiple of > + * this. However, operating on blocks this small is painfully slow, > + * so we also have a transfer size that is larger but only aligned to > + * the smaller block size. > */ > -# define QEMU_MONITOR_MIGRATE_TO_FILE_BS (1024llu * 1024) > +# define QEMU_MONITOR_MIGRATE_TO_FILE_BS 512llu > +# define QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE (1024llu * 1024) > > int qemuMonitorMigrateToFile(qemuMonitorPtr mon, > unsigned int background, > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index e89ad43..5e6cedd 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -1695,10 +1695,15 @@ int qemuMonitorJSONMigrateToFile(qemuMonitorPtr mon, > goto cleanup; > } > > - if (virAsprintf(&dest, "exec:%s | dd of=%s bs=%llu seek=%llu", > - argstr, safe_target, > - QEMU_MONITOR_MIGRATE_TO_FILE_BS, > - offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS) < 0) { > + /* Two dd processes, sharing the same stdout, are necessary to > + * allow starting at an alignment of 512, but without wasting > + * padding to get to the larger alignment useful for speed. */ > + if (virAsprintf(&dest, "exec:%s | { dd bs=%llu seek=%llu if=/dev/null && " > + "dd bs=%llu; } >%s", Does this work with block devices as the target ? We previously switched from cat>> to dd, because it didn't work correctly with block devs. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list