Re: [libvirt] [PATCH] [5/6] Add script hook support to the QEmu driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 26, 2010 at 11:09:08AM -0600, Eric Blake wrote:
> On 03/26/2010 09:45 AM, Daniel Veillard wrote:
> > +    /* now that we know it is about to start call the hook if present */
> > +    if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
> > +        char *xml = virDomainDefFormat(vm->def, 0);
> > +        int hookret;
> > +
> > +        hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
> > +                    VIR_HOOK_QEMU_OP_START, VIR_HOOK_SUBOP_BEGIN, NULL, xml);
> > +        VIR_FREE(xml);
> > +
> > +        /*
> > +         * If the script raised an error abort the launch
> > +         */
> > +        if (hookret < 0)
> > +            goto cleanup;
> 
> Should we also report an error if virHookCall returned 1 because the
> hook could not be run?

I prefer no, that's a system setup error, and I'm afraid of this
breaking everything. That's the danger of this cript extension, while
errors reported by running the script must be taken into account,
failing to run the script should not go in the way.

> > +        /* we can't stop the operation even if the script raised an error */
> > +        virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
> > +                    VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, NULL, xml);
> 
> Likewise, should we report if virHookCall returns non-zero, even though
> we don't abort the operation?

  Any system error would be reported from within virHookCall() itself,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]