[libvirt] [PATCH v3 1/4] numa: describe siblings distances within cells

Martin Kletzander mkletzan at redhat.com
Mon Sep 4 10:40:48 UTC 2017


On Mon, Sep 04, 2017 at 11:51:21AM +0200, Wim ten Have wrote:
>On Mon, 4 Sep 2017 08:49:33 +0200
>Martin Kletzander <mkletzan at redhat.com> wrote:
>
>> On Fri, Sep 01, 2017 at 04:31:50PM +0200, Wim ten Have wrote:
>> >On Thu, 31 Aug 2017 16:36:58 +0200
>> >Martin Kletzander <mkletzan at redhat.com> wrote:
>> >
>> >> >diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
>> >> >index c21d11d..8d804a1 100644
>> >> >--- a/src/conf/cpu_conf.c
>> >> >+++ b/src/conf/cpu_conf.c
>> >> >@@ -642,7 +642,7 @@ virCPUDefFormatBufFull(virBufferPtr buf,
>> >> >     if (virCPUDefFormatBuf(&childrenBuf, def, updateCPU) < 0)
>> >> >         goto cleanup;
>> >> >
>> >> >-    if (virDomainNumaDefCPUFormat(&childrenBuf, numa) < 0)
>> >> >+    if (virDomainNumaDefCPUFormatXML(&childrenBuf, numa) < 0)
>> >> >         goto cleanup;
>> >
>> >> Changing function names should be separate patch.  Why is this
>> >> changed anyway?
>> >
>> >I renamed virDomainNumaDefCPUFormat() to virDomainNumaDefCPUFormatXML()
>> >to make it consistent with already existing function names like
>> >    virDomainNumaDefCPUParseXML()
>> >
>>
>> Then put it in a separate patch.
>
>Sure. Do you advise me to put this patch in same or in a separated set?
>

Whatever suits you, I usually put clean-ups in the series as first
patches so that it is cleanly prepared for the actual changes.  But it's
only a matter of not doing multiple things in one patch in case someone
would be targetting one change in the future (finding a regression,
back-porting it, reverting it).  It also reads a bit more nicely.

>- Wim.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170904/f54ceef9/attachment-0001.sig>


More information about the libvir-list mailing list