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

Martin Kletzander mkletzan at redhat.com
Tue Jul 25 13:48:44 UTC 2017


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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170725/7d07ca3d/attachment-0001.sig>


More information about the libvir-list mailing list