Re: [PATCH 06/11] hyperv: attach virtual disks when defining domains
On Nov 26, 2020, at 9:48 AM, Daniel P. Berrangé
<berrange(a)redhat.com> wrote:
On Tue, Nov 24, 2020 at 02:48:35PM -0500, Matt Coleman wrote:
> + g_autofree char *addressString = g_strdup_printf("%u",
disk->info.addr.drive.unit);
Validate disk->info.type == DRIVE before accessing this field otherwise
the union contents is undefined.
> + if (hypervSetEmbeddedProperty(volumeResource,
"ResourceType", resourceType) < 0)
> + return -1;
> +
> + if (hypervSetEmbeddedProperty(volumeResource, "ResourceSubType",
> + "Microsoft:Hyper-V:Virtual Hard Disk")
< 0)
> + return -1;
The disk->device can be disk, or cdrom or floppy. So if you're hadcoding disk,
then
validate disk->device matches.
The logic in hypervDomainAttachStorageVolume() ensures that it only
calls hypervDomainAttachVirtualDisk() if disk->device is equal to
VIR_DOMAIN_DISK_DEVICE_DISK.
Should I add another check inside hypervDomainAttachVirtualDisk() or is
it acceptable to validate within hypervDomainAttachStorageVolume() as
long as no other functions call hypervDomainAttachVirtualDisk()?
If this is acceptable, I could also validate disk->info.type inside
hypervDomainAttachStorageVolume(), which would cover the other two
critiques.
> + VIR_DEBUG("Now attaching disk image '%s' with
address %d to bus %d of type %d",
> + disk->src->path, disk->info.addr.drive.unit,
disk->info.addr.drive.controller, disk->bus);
Don't access the disk->info.addr until its type is validated