Re: [libvirt] [PATCH 3/6] Implement virDomainUpdateDeviceFlags API in all drivers with media change

On 03/22/2010 01:05 PM, Daniel P. Berrange wrote: > To allow the new virDomainUpdateDeviceFlags() API to be universally > used with all drivers, this patch adds an impl to all the current > drivers which support CDROM or Floppy disk media change via the > current virDomainAttachDeviceFlags API > > + switch (dev->type) { > + case VIR_DOMAIN_DEVICE_DISK: ... > + > + switch (dev->data.disk->device) { > + case VIR_DOMAIN_DISK_DEVICE_CDROM: > + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: > + ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk); > + if (ret == 0) > + dev->data.disk = NULL; > + break; > + > + > + default: > + qemuReportError(VIR_ERR_NO_SUPPORT, > + _("disk bus '%s' cannot be updated."), > + virDomainDiskBusTypeToString(dev->data.disk->bus)); > + break; This break only exits the innermost switch... > + } > + > + if (ret != 0 && cgroup) { > + virCgroupDenyDevicePath(cgroup, > + dev->data.disk->src); > + } > + break; ...is it still safe to call this after reporting an error like that, before breaking from the outermost switch?

> -static int vboxDomainAttachDevice(virDomainPtr dom, const char *xml) { > +static int vboxDomainAttachDeviceImpl(virDomainPtr dom, const char *xml, int mediaChangeOnly ATTRIBUTE_UNUSED) { 80 columns? > + /* Only live config can be changed if xendConfigVersion < 3 */ > + if (priv->xendConfigVersion < 3 && > + (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT || > + flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) { > + virXendError(domain->conn, VIR_ERR_OPERATION_INVALID, "%s", > + _("Xend version does not support modifying " > + "persisted config")); I think 'persistent' sounds better than 'persisted' as an adjective (multiple instances).