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

Re: [libvirt] [PATCH 2/3] snapshot: save domain description with snapshot



Hello Eric,

Am Samstag 13 August 2011 00:08:11 schrieb Eric Blake:
> On 04/12/2011 12:16 AM, Philipp Hahn wrote:
> > Save the domain description with the XML snapshot data.
> > TODOs:
> > - XML file is no longer nicely indented
>
> Cosmetic, and can be fixed later.
>
> > - Fix esx driver
> > - Fix vbox driver
>
> Do these need to save domain xml state in the first place?  They aren't
> using libvirt to track domain state in the first time, but call out to
> the hypervisor for everything.  And if the hypervisor is already doing a
> good job of reverting across configuration changes, then it doesn't hurt
> if they continue to use just <domain>/<uuid> instead of full <domain> in
> the snapshot output that libvirt generates on virDomainSnapshotGetXMLDesc.

I don't have access to any ESX system, so I couldn't check. At least with our 
(very) old VMWare Server when doing a snapshot, the configuration is saved, 
so on revert you get the old configuration again. That difference was 
actually what got us to implement this for Qemu as well.
VBox I didn't check: We're using it for another project I'm currently not 
working on, but there libvirt isn't used to manage it.

> > @@ -8694,9 +8705,17 @@ char *virDomainSnapshotDefFormat(char
> > *domain_uuid, }
> >       virBufferVSprintf(&buf, "<creationTime>%ld</creationTime>\n",
> >                         def->creationTime);
> > -    virBufferAddLit(&buf, "<domain>\n");
> > -    virBufferVSprintf(&buf, "<uuid>%s</uuid>\n", domain_uuid);
> > -    virBufferAddLit(&buf, "</domain>\n");
> > +    if (def->dom != NULL) {
> > +        xml = virDomainDefFormat(def->dom, VIR_DOMAIN_XML_INACTIVE |
> > VIR_DOMAIN_XML_SECURE);
>
> Security hole.  You cannot blindly add VIR_DOMAIN_XML_SECURE if this is
> destined to external output, rather, it has to be passed in from the
> user's flags, and libvirt.c has to validate that
> virDomainSnapshotGetXMLDesc rejects the flag on read-only connections.

Yes, but for a PoC that was the easiest thing to do.  Glad you spotted that.

Sincerely
Philipp Hahn
-- 
Philipp Hahn           Open Source Software Engineer      hahn univention de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/

Attachment: signature.asc
Description: This is a digitally signed message part.


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