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@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
