From b7e73585a8d96677695a52bafb156f26cbd48fb5 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 27 Aug 2014 22:03:04 -0600 Subject: [PATCH] blockcopy: allow block device destination To date, anyone performing a block copy and pivot ends up with the destination being treated as . While this works for data access for a block device, it has at least one noticeable shortcoming: virDomainGetBlockInfo() reports allocation differently for block devices visited as files (the size of the device) than for block devices visited as (the maximum sector used, as reported by qemu); and this difference is significant when trying to manage qcow2 format on block devices that can be grown as needed. Of course, the more powerful virDomainBlockCopy() API can already express the ability to set the type. But a new API can't be backported, while a new flag to an existing API can; and it is also rather inconvenient to have to resort to the full power of generating XML when just adding a flag to the older call will do the trick. So this patch enhances blockcopy to let the user flag when the resulting XML after the copy must list the device as type='block'. * include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_REBASE_COPY_DEV): New flag. * src/libvirt.c (virDomainBlockRebase): Document it. * tools/virsh-domain.c (opts_block_copy, blockJobImpl): Add --blockdev option. * tools/virsh.pod (blockcopy): Document it. * src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow new flag. (qemuDomainBlockCopy): Remember the flag, and make sure it is only used on actual block devices. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it. Signed-off-by: Eric Blake --- include/libvirt/libvirt.h.in | 2 ++ src/libvirt.c | 8 +++-- src/qemu/qemu_driver.c | 36 ++++++++++++------- .../qemuxml2argv-disk-mirror.xml | 4 +-- tools/virsh-domain.c | 6 ++++ tools/virsh.pod | 7 ++-- 6 files changed, 45 insertions(+), 18 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6d0c042dfd..aced31c5dd 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2638,6 +2638,8 @@ typedef enum { VIR_DOMAIN_BLOCK_REBASE_RELATIVE = 1 << 4, /* Keep backing chain referenced using relative names */ + VIR_DOMAIN_BLOCK_REBASE_COPY_DEV = 1 << 5, /* Treat destination as block + device instead of file */ } virDomainBlockRebaseFlags; int virDomainBlockRebase(virDomainPtr dom, const char *disk, diff --git a/src/libvirt.c b/src/libvirt.c index b00ee16760..4806535950 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19885,7 +19885,10 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, * pre-create files with relative backing file names, rather than the default * of absolute backing file names; as a security precaution, you should * generally only use reuse_ext with the shallow flag and a non-raw - * destination file. + * destination file. By default, the copy destination will be treated as + * type='file', but using VIR_DOMAIN_BLOCK_REBASE_COPY_DEV treats the + * destination as type='block' (affecting how virDomainGetBlockInfo() will + * report allocation after pivoting). * * A copy job has two parts; in the first phase, the @bandwidth parameter * affects how fast the source is pulled into the destination, and the job @@ -19960,7 +19963,8 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk, virCheckNonNullArgGoto(base, error); } else if (flags & (VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | - VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)) { + VIR_DOMAIN_BLOCK_REBASE_COPY_RAW | + VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) { virReportInvalidArg(flags, _("use of flags in %s requires a copy job"), __FUNCTION__); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 16a89c85f7..4bf754f072 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15372,7 +15372,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm, /* Preliminaries: find the disk we are editing, sanity checks */ virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW | - VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1); + VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | + VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1); priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); @@ -15433,25 +15434,34 @@ qemuDomainBlockCopy(virDomainObjPtr vm, virReportSystemError(errno, _("unable to stat for disk %s: %s"), disk->dst, dest); goto endjob; - } else if (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) { + } else if (flags & (VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | + VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) { virReportSystemError(errno, _("missing destination file for disk %s: %s"), disk->dst, dest); goto endjob; } - } else if (!S_ISBLK(st.st_mode) && st.st_size && - !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("external destination file for disk %s already " - "exists and is not a block device: %s"), - disk->dst, dest); - goto endjob; + } else if (!S_ISBLK(st.st_mode)) { + if (st.st_size && !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("external destination file for disk %s already " + "exists and is not a block device: %s"), + disk->dst, dest); + goto endjob; + } + if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV) { + virReportError(VIR_ERR_INVALID_ARG, + _("blockdev flag requested for disk %s, but file " + "'%s' is not a block device"), disk->dst, dest); + goto endjob; + } } if (VIR_ALLOC(mirror) < 0) goto endjob; /* XXX Allow non-file mirror destinations */ - mirror->type = VIR_STORAGE_TYPE_FILE; + mirror->type = flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV ? + VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE; if (format) { if ((mirror->format = virStorageFileFormatTypeFromString(format)) <= 0) { @@ -15546,7 +15556,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | VIR_DOMAIN_BLOCK_REBASE_COPY | VIR_DOMAIN_BLOCK_REBASE_COPY_RAW | - VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1); + VIR_DOMAIN_BLOCK_REBASE_RELATIVE | + VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1); if (!(vm = qemuDomObjFromDomain(dom))) return -1; @@ -15583,7 +15594,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, } flags &= (VIR_DOMAIN_BLOCK_REBASE_SHALLOW | - VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT); + VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | + VIR_DOMAIN_BLOCK_REBASE_COPY_DEV); ret = qemuDomainBlockCopy(vm, dom->conn, path, base, format, bandwidth, flags); vm = NULL; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml index 46f2a3eef0..7495a45856 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml @@ -17,8 +17,8 @@ - - + +
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2bb3bbc48f..99336d294d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1544,6 +1544,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; if (vshCommandOptBool(cmd, "raw")) flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW; + if (vshCommandOptBool(cmd, "blockdev")) + flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV; if (vshCommandOptStringReq(ctl, cmd, "dest", &base) < 0) goto cleanup; if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0) @@ -1855,6 +1857,10 @@ static const vshCmdOptDef opts_block_copy[] = { .type = VSH_OT_BOOL, .help = N_("use raw destination file") }, + {.name = "blockdev", + .type = VSH_OT_BOOL, + .help = N_("copy destination is block device instead of regular file") + }, {.name = "wait", .type = VSH_OT_BOOL, .help = N_("wait for job to reach mirroring phase") diff --git a/tools/virsh.pod b/tools/virsh.pod index d95df364d8..df198e6c65 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -899,7 +899,8 @@ unlimited. The hypervisor can choose whether to reject the value or convert it to the maximum value allowed. =item B I I I [I] [I<--shallow>] -[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--async>] [I<--verbose>]] +[I<--reuse-external>] [I<--raw>] [I<--blockdev>] +[I<--wait> [I<--async>] [I<--verbose>]] [{I<--pivot> | I<--finish>}] [I<--timeout> B] Copy a disk backing image chain to I. By default, this command @@ -917,7 +918,9 @@ The format of the destination is determined by the first match in the following list: if I<--raw> is specified, it will be raw; if I<--reuse-external> is specified, the existing destination is probed for a format; and in all other cases, the destination format will -match the source format. +match the source format. The destination is treated as a regular +file unless I<--blockdev> is used to signal that it is a block +device. By default, the copy job runs in the background, and consists of two phases. Initially, the job must copy all data from the source, and