On Mon, Mar 22, 2010 at 02:58:16PM -0600, Eric Blake wrote:
> 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?
Yep, this is correct. THe virCgroupDenyDevicePath() function has to be called
on all error scenarios here, both from the "default:" case, and error in the
previous 'VIR_DOMAIN_DISK_DEVICE_FLOPPY:' case, so that it removes the ACL
that was setup immediately before the 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).
This was just a copied diagnostic from code higher up
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list