Re: [PATCH] qemu: fix fail to start vm with one cell use memdev and another not

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

 



On 16.01.2015 10:49, lhuang wrote:
> 
> On 01/16/2015 04:54 PM, Michal Privoznik wrote:
>> On 16.01.2015 03:12, Luyao Huang wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1182467
>>>
>>> We changed our build numa qemu cmd line way in f309db,
>>> But qemu cannot accept one cell use memdev and another not,
>>> so this cause a issue when we start a vm with one cell
>>> have hugpage or nodemask settings and others not.
>>>
>>> After this patch we will use memdev for all cells if there
>>> is at least one cell need this.
>>>
>>> Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
>>> ---
>>>   src/qemu/qemu_command.c                            | 88
>>> +++++++++++++---------
>>>   .../qemuxml2argv-numatune-memnode-no-memory.args   |  3 +-
>>>   2 files changed, 55 insertions(+), 36 deletions(-)
>> While I agree the bug you are trying to fix is real bug, the patch look
>> a bit messy. Moreover, as far as I know Peter is doing work in this area
>> and he's willing to fix the bug. You know, he's rewriting the area from
>> scratch (or something like that), so I don't really want to cause a
>> rebase conflict to him. Moreover, some tests fail after this patch, so I
>> will not merge this for now, sorry.
> 
> Nothing, I just don't know Peter is working on it.
> About patch look a bit messy, i noticed this when i wrote, but i cannot
> found a good way to fix it without rewrite this place.

Yeah, sometimes it's better to split the bug fix into two patches (or
even more). In the first part you reformat the code. Then, in the second
part of the patchset you just fix the bug. It's a bit harder to
backport, but easier to maintain the upstream.

Michal

--
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]