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

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(a)redhat.com&gt; > --- > 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.

Michal