[libvirt] [PATCH] Add function that raises error if domain is not active

[Michal Privoznik]

On 04/13/2018 09:31 AM, Ján Tomko wrote:
> On Thu, Apr 12, 2018 at 07:49:15PM +0000, Clementine Hayat wrote:
>> Add a function named virDomainObjCheckIsActive in src/conf/domain_conf.c.
>> It calls virDomainObjIsActive, raises error and returns.
> 
> *raises error if necessary
> 
>>
>> There is a lot of occurence of this pattern and it will save 3 lines on
>> each call. Knowing that there is over 100 occurences, it will remove 300
>> lines from the code base.
> 
> False advertising.
> 
> If you don't want to send them all in one patch, I suggest spliting them
> per-subdirectory: conf, qemu, libxl, vz, ... (and use that prefix in the
> commit summary)
> 
>>
>> Signed-off-by: Clementine Hayat <clem at lse.epita.fr>
> 
> If you have any accents in your name, feel free to use them. Even danpb
> recently decided the world is ready for UTF-8:
> https://libvirt.org/git/?p=libvirt.git;a=search;s=Daniel+P.+Berrang%C3%A9;st=author
> 
> 
>> ---
>> Patch proposed for gsoc2018.
>>
>> src/conf/domain_conf.c   | 11 +++++
>> src/conf/domain_conf.h   |  2 +
>> src/libvirt_private.syms |  1 +
>> src/qemu/qemu_driver.c   | 96 +++++++++-------------------------------
>> 4 files changed, 34 insertions(+), 76 deletions(-)
>>
> 
> The changes look good but the patch feels incomplete.

I was just looking at this patch. Yes it is incomplete but I think that
was the point. Give upstream taste what the patch looks like before
diving in and changing all those 108 occurrences (I did change them
locally).

The patch looks good to me, but as Jan suggests, you can break this big
change into smaller (=easier to review) patches. In the first you can
just introduce the function. And then have patch per each folder.

Alternatively, you can write a small semantic patch [1] and use that to
generate the diff. But this is rather advanced stuff.

Michal

1: http://coccinelle.lip6.fr/