Google
  Web www.spinics.net

Re: [PATCH 4/6] conf: HostdevDef parse/format helper functions

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


On 02/20/2012 10:10 AM, Laine Stump wrote:
> In an upcoming patch, virDomainNetDef will acquire a
> virDomainHostdevDef, and the <interface> XML will take on some of the
> elements of a <hostdev>. To avoid duplicating the code for parsing and
> formatting the <source> element (which will be nearly identical in
> these two cases), this patch factors those parts out of the
> HostdevDef's parse and format functions, and puts them into separate
> helper functions that are now called by the HostdevDef
> parser/formatter, and will soon be called by the NetDef
> parser/formatter.
> 
> One change in behavior - previously virDomainHostdevDefParseXML() had
> diverged from current common coding practice by logging an error and
> failing if it found any subelements of <hostdev> other than those it
> understood (standard libvirt practice is to ignore/discard unknown
> elements and attributes during parse). The new helper function ignores
> unknown elements, and thus so does the new
> virDomainHostdevDefParseXML.

Seems like a reasonable change in behavior.

>  
> +static int
> +virDomainHostdevPartsParse(xmlNodePtr node,
> +                           xmlXPathContextPtr ctxt,
> +                           char *mode, char *type,

I'd use 'const char *' for both of these parameters.

> +
> +    /* @type is passed in from the caller rather than read from the
> +     * xml document, because it is specificed in different places for

s/specificed/specified/

ACK with those nits fixed.

At this point, I'm comfortable if you feel like pushing patches 1-4
(obviously, with the v2 fixes to patch 1) to get them out of the way, as
they seem like reasonable cleanups whether or not the rest of your code
is complete, and doing it now will reduce the likelihood of merge
conflicts if someone else touches the same areas of code.  But it's
probably better to hold off on 5 and 6 until we have wired up the code
to use the refactorization.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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

[Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]

Powered by Linux

Google
  Web www.spinics.net