[PATCH 06/11] hyperv: attach virtual disks when defining domains

[Matt Coleman]

> On Nov 26, 2020, at 9:48 AM, Daniel P. Berrangé <berrange at 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