Re: [libvirt] [PATCH 1/2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls
On 04/13/2015 07:10 AM, Peter Krempa wrote:
On Fri, Apr 10, 2015 at 12:41:12 +0200, Erik Skultety wrote:
> This patch adds checks for empty bitmaps right after the calls of
> virBitmapParse. These only include spots where set API's are called and
> where domain's XML is parsed.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1210545
> ---
> src/conf/domain_conf.c | 35 +++++++++++++++++++++++++++++++----
> src/conf/numa_conf.c | 23 +++++++++++++++++++----
> src/qemu/qemu_driver.c | 5 +++--
> src/xenconfig/xen_sxpr.c | 7 +++++++
> 4 files changed, 60 insertions(+), 10 deletions(-)
I've "git grep"'d a few uses of this function and found a few places
that would also need a chceck. Few examples are the XML parser for
networks, the use in qemuProcessStart() to parse the automatic
placemenet and so on ..
The rest looks good though.
Peter
At the moment, there's really no need to check the bitmap 'class_id' in
network.conf, because it's a status XML anyway, however even if user
changed our status XML and an error occurred, it would be user's
responsibility, the only exception to this would be a daemon crash which
isn't this case. Moreover, if you parse the bitmap "0,^0" the empty
bitmap wouldn't have almost any effect, because by default first 3 bits
are always set during network object creation.
To the qemuProcessStart, we call numad to get a suggestion for the
nodeset and the only problem would be if numad returned empty string,
however this would be handled by virBitmapParse itself successfully.
The other occurrences are tests mostly, v1 included this check for XEN
parser, I'm not sure about the 'Parallels' though.
Anyway, I squashed the second patch into this one as you suggested and
modified an existing test in virbitmaptest.c for v2. (I thought checking
this specific case wasn't worth having a separate test, so I modified
bitmap-overlap test, instead of just testing the bitmap parsing.)
Erik