On Wed, Oct 19, 2011 at 04:42:58PM +0200, Michal Privoznik wrote:
> This patch implements on_missing feature in qemu driver.
> Upon qemu startup process an accessibility of CDROMs
> and floppy disks is checked. The source might get dropped
> if unavailable and on_missing is set accordingly.
> No event is emit thought. Look for follow up patch.
> ---
> src/qemu/qemu_domain.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 4 +++
> src/qemu/qemu_process.c | 4 +++
> 3 files changed, 74 insertions(+), 0 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 5abc900..ffcee29 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1614,3 +1614,69 @@ qemuDomainSetFakeReboot(struct qemud_driver *driver,
> if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
> VIR_WARN("Failed to save status on vm %s", vm->def->name);
> }
> +
> +int
> +qemuDomainCheckDiskPresence(struct qemud_driver *driver,
> + virDomainObjPtr vm,
> + bool being_migrated)
We do this check upon restore from save image/snapshot too,
so 'being_migrated' is a little misleading. How about
'bool start_with_state' to indicate a start attempt that
is using existing VM state.
> +{
> + int ret = -1;
> + int i;
> + int accessRet;
> + virDomainDiskDefPtr disk;
> + char uuid[VIR_UUID_STRING_BUFLEN] ATTRIBUTE_UNUSED;
> +
> + virUUIDFormat(vm->def->uuid, uuid);
> +
> + for (i = 0; i < vm->def->ndisks; i++) {
> + disk = vm->def->disks[i];
> +
> + if (!disk->on_missing || !disk->src)
> + continue;
> +
> + if ((accessRet = virFileAccessibleAs(disk->src, F_OK,
> + driver->user,
> + driver->group)) >= 0) {
> + /* disk accessible or virFileAccessibleAs()
> + * terminated with signal*/
> + continue;
> + }
You can't assume 'disk->src' is valid. This struct field is
only valid for disks type=file|block. It will SEGV if the
type=network (ie NBD/Ceph/Sheepdog.
> +
> + switch (disk->on_missing) {
> + case VIR_DOMAIN_ON_MISSING_OPTIONAL:
> + break;
> +
> + case VIR_DOMAIN_ON_MISSING_MANDATORY:
> + virReportSystemError(-accessRet,
> + _("cannot access file '%s'"),
> + disk->src);
> + goto cleanup;
> + break;
> +
> + case VIR_DOMAIN_ON_MISSING_REQUISITE:
> + if (!being_migrated) {
> + virReportSystemError(-accessRet,
> + _("cannot access file '%s'"),
> + disk->src);
> + goto cleanup;
> + }
> + break;
> +
> + default:
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("something is tragically wrong"));
> + goto cleanup;
> + }
> +
> + VIR_DEBUG("Droping disk '%s' on domain '%s' (UUID '%s') "
> + "due to not accessible source '%s'",
> + disk->dst, vm->def->name, uuid, disk->src);
> +
> + VIR_FREE(disk->src);
> + }
> +
> + ret = 0;
> +
> +cleanup:
> + return ret;
> +}
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a7fe86c..2f1d94d 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2880,6 +2880,10 @@ int qemuProcessStart(virConnectPtr conn,
> NULL) < 0)
> goto cleanup;
>
> + VIR_DEBUG("Checking for CDROM and floppy presence");
> + if (qemuDomainCheckDiskPresence(driver, vm, migrateFrom != NULL) < 0)
> + goto cleanup;
This should cope with restore from save file, and incoming migration.
Does it also work with restore from snapshot though ?
> +
> /* If you are using a SecurityDriver with dynamic labelling,
> then generate a security label for isolation */
> VIR_DEBUG("Generating domain security label (if required)");
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list