[libvirt] [PATCH v2 1/2] qemu: make attaching disk partition to VM illegal

Michal Privoznik mprivozn at redhat.com
Wed Oct 2 08:45:28 UTC 2019


On 9/30/19 3:41 PM, Pavel Mores wrote:
> The way in which the qemu driver generates aliases for disks involves
> ignoring the partition number part of a target dev name.  This means that
> all partitions of a block device and the device itself all end up with the
> same alias.  If multiple such disks are specified in XML, the resulting
> name clash makes qemu invocation fail.
> 
> Since attaching partitions to qemu VMs doesn't seem to make much sense
> anyway, disallow partitions in target specifications altogether.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1346265
> 
> Signed-off-by: Pavel Mores <pmores at redhat.com>
> ---
>   src/qemu/qemu_domain.c                        | 15 +++++++++++
>   .../disk-attaching-partition-nosupport.xml    | 27 +++++++++++++++++++
>   tests/qemuxml2argvtest.c                      |  1 +
>   3 files changed, 43 insertions(+)
>   create mode 100644 tests/qemuxml2argvdata/disk-attaching-partition-nosupport.xml
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e8e895d9aa..545c985f40 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5880,6 +5880,8 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
>   {
>       const char *driverName = virDomainDiskGetDriver(disk);
>       virStorageSourcePtr n;
> +    int idx;
> +    int partition;
>   
>       if (disk->src->shared && !disk->src->readonly &&
>           !qemuBlockStorageSourceSupportsConcurrentAccess(disk->src)) {
> @@ -5948,6 +5950,19 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
>           return -1;
>       }
>   
> +    if (virDiskNameParse(disk->dst, &idx, &partition) < 0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("invalid disk target '%s'"), disk->dst);
> +        return -1;
> +    }
> +
> +    if (partition != 0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("can't attach disk partition '%s', please attach whole disk instead"),
> +                       disk->dst);

Hopefully it's not too late, but this error message and the commit title 
neither don't look okay. You can attach /dev/sda1 to a domain, but we 
don't want you to put "sda1" into the disk target, we want you to name 
it just "sda". So perhaps "Refuse partitions in disk targets" as commit 
tile and "invalid disk target '%s', partitions can't appear in disk 
targets" for the error message looks better?

The patch looks goot otherwise. Currently we are in a freeze, but since 
this is a bug fix it can go in. Objections anybody? I volunteer to merge it.

Michal




More information about the libvir-list mailing list