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

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

 



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@xxxxxxxxx>
---
  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




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

  Powered by Linux