Re: [libvirt] libvirt disk labels and QEMU drive host alias generation
On Tue, Jun 14, 2016 at 12:01:52PM +0100, Daniel P. Berrange wrote:
On Tue, Jun 14, 2016 at 12:50:09PM +0200, Kashyap Chamarthy wrote:
> Summary
> -------
>
> It seems like libvirt is generating the same QEMU drive host
> alias ('drive-virtio-disk%d') for different two disk labels (vdb, vdb1).
>
> [Refer the contextual root cause analysis discussion at the end with
> Laine & Peter.]
>
> Let's take a quick example to demonstrate the issue.
>
> On a guest that is shut down, attach a couplee of disks with labels
> 'vdb', and 'vdb1'
>
> $ sudo virsh attach-disk cvm1 \
> /export/vmimages/1.raw vdb --config
>
> $ sudo virsh attach-disk cvm1 \
> /export/vmimages/1.raw vdb1 --config
That second command is user error - hotplugging a disk partition
is nonsensical, except with Xen which allows that for paravirtualized
disks. We should reject such disk names with an error right away in
all other cases.
Okay, filed this upstream bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1346265 --
Disallow disk hot-plugging with invalid target labels (e.g. disk
partition number for non-paravirt disks)
I realized supplying disk partition numbers as targets is not sensible,
but virDiskNameParse() parses partition numbers too:
$ less -N src/util/virutil.c
[...]
543 /* Translates a device name of the form (regex) /^[fhv]d[a-z]+[0-9]*$/
544 * into the corresponding index and partition number
545 * (e.g. sda0 => (0,0), hdz2 => (25,2), vdaa12 => (26,12))
546 * @param name The name of the device
547 * @param disk The disk index to be returned
548 * @param partition The partition index to be returned
549 * @return 0 on success, or -1 on failure
550 */
551 int virDiskNameParse(const char *name, int *disk, int *partition)
552 {
[...]
--
/kashyap