[libvirt] [PATCH for 1.2.6] qemu: copy: Accept 'format' parameter when copying to a non-existing img
We have the following matrix of possible arguments handled by the logic statement touched by this patch: | flags & _REUSE_EXT | !(flags & _REUSE_EXT) -------+--------------------+---------------------- format| (1) | (2) -------+--------------------+---------------------- !format| (3) | (4) -------+--------------------+---------------------- In cases 1 and 2 the user provided a format, in cases 3 and 4 not. The user requests to use a pre-existing image in 1 and 3 and libvirt shall create a new image in 2 and 4. The difference between cases 3 and 4 is that for 3 the format is probed from the user-provided image, whereas in 4 we just use the existing disk format. The current code would treat cases 1,3 and 4 correctly but in case 2 the format provided by the user would be ignored. The particular piece of code was broken in commit 35c7701c64508f975dfeb8 but since it was introduced a few commits before that it was never released as working. --- src/qemu/qemu_driver.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 775f6ab..d45a161 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15356,29 +15356,34 @@ qemuDomainBlockCopy(virDomainObjPtr vm, /* XXX Allow non-file mirror destinations */ mirror->type = VIR_STORAGE_TYPE_FILE; + if (format) { + if ((mirror->format = virStorageFileFormatTypeFromString(format)) <= 0) { + virReportError(VIR_ERR_INVALID_ARG, _("unrecognized format '%s'"), + format); + goto endjob; + } + } else { + if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) { + mirror->format = disk->src->format; + } else { + /* If the user passed the REUSE_EXT flag, then either they + * also passed the RAW flag (and format is non-NULL), or it is + * safe for us to probe the format from the file that we will + * be using. */ + mirror->format = virStorageFileProbeFormat(dest, cfg->user, + cfg->group); + } + } + + /* pre-create the image file */ if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) { int fd = qemuOpenFile(driver, vm, dest, O_WRONLY | O_TRUNC | O_CREAT, &need_unlink, NULL); if (fd < 0) goto endjob; VIR_FORCE_CLOSE(fd); - if (!format) - mirror->format = disk->src->format; - } else if (format) { - mirror->format = virStorageFileFormatTypeFromString(format); - if (mirror->format <= 0) { - virReportError(VIR_ERR_INVALID_ARG, _("unrecognized format '%s'"), - format); - goto endjob; - } - } else { - /* If the user passed the REUSE_EXT flag, then either they - * also passed the RAW flag (and format is non-NULL), or it is - * safe for us to probe the format from the file that we will - * be using. */ - mirror->format = virStorageFileProbeFormat(dest, cfg->user, - cfg->group); } + if (!format && mirror->format > 0) format = virStorageFileFormatTypeToString(mirror->format); if (VIR_STRDUP(mirror->path, dest) < 0) -- 1.9.3
On 07/01/2014 09:31 AM, Peter Krempa wrote:
We have the following matrix of possible arguments handled by the logic statement touched by this patch: | flags & _REUSE_EXT | !(flags & _REUSE_EXT) -------+--------------------+---------------------- format| (1) | (2) -------+--------------------+---------------------- !format| (3) | (4) -------+--------------------+----------------------
In cases 1 and 2 the user provided a format, in cases 3 and 4 not. The user requests to use a pre-existing image in 1 and 3 and libvirt shall
s/shall/will/
create a new image in 2 and 4.
The difference between cases 3 and 4 is that for 3 the format is probed from the user-provided image, whereas in 4 we just use the existing disk format.
With the further caveat that if the _SHALLOW flag was requested, we are guaranteed the probe is safe (a shallow image implies a backing file, but raw cannot have a backing file, and probing is only a security hole if raw is probed)
The current code would treat cases 1,3 and 4 correctly but in case 2 the format provided by the user would be ignored.
The particular piece of code was broken in commit 35c7701c64508f975dfeb8 but since it was introduced a few commits before that it was never released as working. --- src/qemu/qemu_driver.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-)
ACK. I'll take care of pushing the fix to the affected maint branches, but yours needs to go in for 1.2.6. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 07/01/14 17:42, Eric Blake wrote:
On 07/01/2014 09:31 AM, Peter Krempa wrote:
We have the following matrix of possible arguments handled by the logic statement touched by this patch: | flags & _REUSE_EXT | !(flags & _REUSE_EXT) -------+--------------------+---------------------- format| (1) | (2) -------+--------------------+---------------------- !format| (3) | (4) -------+--------------------+----------------------
In cases 1 and 2 the user provided a format, in cases 3 and 4 not. The user requests to use a pre-existing image in 1 and 3 and libvirt shall
s/shall/will/
create a new image in 2 and 4.
The difference between cases 3 and 4 is that for 3 the format is probed from the user-provided image, whereas in 4 we just use the existing disk format.
With the further caveat that if the _SHALLOW flag was requested, we are guaranteed the probe is safe (a shallow image implies a backing file, but raw cannot have a backing file, and probing is only a security hole if raw is probed)
The current code would treat cases 1,3 and 4 correctly but in case 2 the format provided by the user would be ignored.
The particular piece of code was broken in commit 35c7701c64508f975dfeb8 but since it was introduced a few commits before that it was never released as working. --- src/qemu/qemu_driver.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-)
ACK. I'll take care of pushing the fix to the affected maint branches, but yours needs to go in for 1.2.6.
Pushed; Thanks. Peter
participants (2)
-
Eric Blake -
Peter Krempa