[PATCH 2/4] qemu_domainjob: job funcitons moved out of `qemu_domain`

Michal Privoznik mprivozn at redhat.com
Fri Jul 10 14:36:02 UTC 2020


On 7/10/20 9:11 AM, Prathamesh Chavan wrote:
> We move functions `qemuDomainObjPrivateXMLParseJob` and
> `qemuDomainObjPrivateXMLFormatJob` to `qemu_domainjob`.
> To make this happen, corresponding changes in their
> parameters were made, as well as they were exposed
> to be used in `qemu_domain` file.
> 
> Signed-off-by: Prathamesh Chavan <pc44800 at gmail.com>
> ---
>   src/qemu/qemu_domain.c    | 245 +-------------------------------------
>   src/qemu/qemu_domainjob.c | 244 +++++++++++++++++++++++++++++++++++++
>   src/qemu/qemu_domainjob.h |   8 ++
>   3 files changed, 254 insertions(+), 243 deletions(-)
> 


>   static bool
>   qemuDomainHasSlirp(virDomainObjPtr vm)
>   {
> @@ -2399,7 +2303,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
>       if (priv->lockState)
>           virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState);
>   
> -    if (qemuDomainObjPrivateXMLFormatJob(buf, vm, priv) < 0)
> +    if (qemuDomainObjPrivateXMLFormatJob(buf, vm) < 0)

This means, that this patch is not a plain code movement. Just for 
future work, break patches into logical/semantic changes. For instance 
in this case it could have been: one patch that drops the @priv argument 
(since the function already gets @vm from which it can obtain @priv), 
the other patch would be actual code movement.

I know it probably doesn't make much sense and I totally get it. I did 
not make any sense to me when I was starting with libvirt - the end 
result is the same, isn't it? What's the fuss then? but then I started 
reviewing patches and realized very quickly how important it is. And 
then I started backporting patches to RHEL and I realized it again :-)

But as you'll see in patch 3/4 we probably don't need to move everything.

Michal




More information about the libvir-list mailing list