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

Re: [libvirt] [PATCH 2/4] qemu: Save qemu driver in qemuDomainObjPrivateData



On Tue, Jul 25, 2017 at 03:20:48PM +0200, Michal Privoznik wrote:
On 07/24/2017 04:09 PM, Martin Kletzander wrote:
This way we can finally make it static and not use any externs anywhere.

Signed-off-by: Martin Kletzander <mkletzan redhat com>
---
 src/qemu/qemu_domain.c  | 3 ++-
 src/qemu/qemu_domain.h  | 2 ++
 src/qemu/qemu_driver.c  | 2 +-
 src/qemu/qemu_process.c | 5 +----
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f1e144d92b64..0b7c45280321 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1662,7 +1662,7 @@ qemuDomainClearPrivatePaths(virDomainObjPtr vm)


 static void *
-qemuDomainObjPrivateAlloc(void *opaque ATTRIBUTE_UNUSED)
+qemuDomainObjPrivateAlloc(void *opaque)
 {
     qemuDomainObjPrivatePtr priv;

@@ -1679,6 +1679,7 @@ qemuDomainObjPrivateAlloc(void *opaque ATTRIBUTE_UNUSED)
         goto error;

     priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
+    priv->driver = opaque;

     return priv;

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 365b23c96167..71567af034f5 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -217,6 +217,8 @@ struct _qemuDomainSecretInfo {
 typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
 typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
 struct _qemuDomainObjPrivate {
+    virQEMUDriverPtr driver;
+
     struct qemuDomainJobObj job;

     virBitmapPtr namespaces;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6568def156f4..9c54571cf078 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -159,7 +159,7 @@ static int qemuGetDHCPInterfaces(virDomainPtr dom,
                                  virDomainObjPtr vm,
                                  virDomainInterfacePtr **ifaces);

-virQEMUDriverPtr qemu_driver = NULL;
+static virQEMUDriverPtr qemu_driver;


 static void
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 525521aaf0ca..757f5d95e0b7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -120,9 +120,6 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
 }


-/* XXX figure out how to remove this */
-extern virQEMUDriverPtr qemu_driver;
-
 /*
  * This is a callback registered with a qemuAgentPtr instance,
  * and to be invoked when the agent console hits an end of file
@@ -518,9 +515,9 @@ qemuProcessHandleReset(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 static void
 qemuProcessFakeReboot(void *opaque)
 {
-    virQEMUDriverPtr driver = qemu_driver;
     virDomainObjPtr vm = opaque;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    virQEMUDriverPtr driver = priv->driver;

Well, storing driver in private data looks like overkill for this
purpose. qemuProcessFakeReboot() runs in a thread (which explains weird
function arguments). But in order to pass qemu driver into this function
we can make the function take a struct.
On the other hand, it might come handy (even right now) as it enables us
to clean up some code where we already have both priv and driver in
function arguments. Frankly, I'm torn. So it's up to you whether you
want to go this way or the one I'm suggesting.


I'm perfectly fine with passing the struct and I have no problem with
changing this that way.  The clean up of qemuProcessFakeReboot is not the
main purpose of this clean up; it's the last patch and enabling future
clean ups.  However, I have thought about it and when I pass the struct,
it will eventually still get removed and it will end up in this form
during the future clean ups.  In other words, I can pass the struct, but
it's not needed any more since the driver will be available even without
that =)

So I'll stick with this since you left me decide ;)

Michal

Attachment: signature.asc
Description: Digital signature


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