1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/4] Clean up machineName-related code
  4. View patch

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

Martin Kletzander posted 4 patches 7 years, 9 months ago
[libvirt] [PATCH 2/4] qemu: Save qemu driver in qemuDomainObjPrivateData
Posted by Martin Kletzander 7 years, 9 months ago 
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;
     virObjectEventPtr event = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED;
-- 
2.13.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: Save qemu driver in qemuDomainObjPrivateData
Posted by Michal Privoznik 7 years, 9 months ago 
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.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: Save qemu driver in qemuDomainObjPrivateData
Posted by Martin Kletzander 7 years, 9 months ago 
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: Save qemu driver in qemuDomainObjPrivateData
Posted by Michal Privoznik 7 years, 9 months ago 
On 07/25/2017 03:48 PM, Martin Kletzander wrote:
> 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 ;)

Right. We don't need both approaches. ACK to this and to 1/4 too then.
Although, let me just point out that it was difficult for me to track
that it is indeed driver that is passed as @opaque to
qemuDomainObjPrivateAlloc(). It isn't visible at the first glance.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.