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

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


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