On 07/31/2013 09:51 AM, Guannan Ren wrote:
> Go through disks of guest, if one disk doesn't exist or its backing
> chain is broken, with 'optional' startupPolicy, for CDROM and Floppy
> we only discard its source path definition in xml, for disks we drop
> it from disk list and free it.
> ---
> include/libvirt/libvirt.h.in | 1 +
> src/qemu/qemu_domain.c | 77 +++++++++++++++++++++++++++++++++++---------
> 2 files changed, 62 insertions(+), 16 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index c0eb25b..3888ba5 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -4728,6 +4728,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn,
> */
> typedef enum {
> VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */
> + VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START = 1,
s/HARDDISK/DISK/ to make it consistent
>
> #ifdef VIR_ENUM_SENTINELS
> VIR_DOMAIN_EVENT_DISK_CHANGE_LAST
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 1ff802c..5da6e18 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2027,13 +2027,59 @@ cleanup:
> }
>
> static int
> +qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainDiskDefPtr disk,
> + size_t *nextdisk)
> +{
> + char uuid[VIR_UUID_STRING_BUFLEN];
> + virDomainEventPtr event = NULL;
> + virDomainDiskDefPtr del_disk = NULL;
> +
> + virUUIDFormat(vm->def->uuid, uuid);
> +
> + VIR_DEBUG("Dropping disk '%s' on domain '%s' (UUID '%s') "
> + "due to inaccessible source '%s'",
> + disk->dst, vm->def->name, uuid, disk->src);
> +
> + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
> + disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
> +
> + event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL,
> + disk->info.alias,
> + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
> + /* For cdrom or floppy, we only remove its source definition
> + * So the nextdisk need to point to next disk.
> + */
> + (*nextdisk)++;
> + VIR_FREE(disk->src);
> + } else {
> + event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL,
> + disk->info.alias,
> + VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START);
> +
> + if (!(del_disk = virDomainDiskRemoveByName(vm->def, disk->src))) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("no source device %s"), disk->src);
> + return -1;
> + }
> + virDomainDiskDefFree(del_disk);
> + }
> +
> + if (event)
> + qemuDomainEventQueue(driver, event);
> +
> + return 0;
> +}
> +
> +static int
> qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainDiskDefPtr disk,
> - bool cold_boot)
> + bool cold_boot,
> + size_t *nextdisk)
> {
> char uuid[VIR_UUID_STRING_BUFLEN];
> - virDomainEventPtr event = NULL;
> int startupPolicy = disk->startupPolicy;
>
> virUUIDFormat(vm->def->uuid, uuid);
> @@ -2057,16 +2103,8 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
> }
>
> virResetLastError();
This should really be one function up, especially when I requested it as
a condition for ACK.
> - VIR_DEBUG("Dropping disk '%s' on domain '%s' (UUID '%s') "
> - "due to inaccessible source '%s'",
> - disk->dst, vm->def->name, uuid, disk->src);
> -
> - event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL, disk->info.alias,
> - VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
> - if (event)
> - qemuDomainEventQueue(driver, event);
> -
> - VIR_FREE(disk->src);
> + if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk, nextdisk) < 0)
> + goto error;
>
> return 0;
>
> @@ -2082,21 +2120,28 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
> int ret = -1;
> size_t i;
> virDomainDiskDefPtr disk;
> + size_t count = vm->def->ndisks;
> + size_t nextdisk = 0;
> +
Definitely drop all this nextdisk nonsense, you can clean it up with:
ssize_t i;
for(i = ndisks - 1; i >= 0; i--) ...
then you don't have to pass it to any underlying function, or you can
pass 'i' and call virDomainDiskRemove() instead of RemoveByName.
looking forward to v2 (or v6),
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list