On 08/15/2017 10:42 PM, John Ferlan wrote:
>
>
> On 08/15/2017 11:32 AM, Michal Privoznik wrote:
>> On 07/26/2017 05:05 PM, John Ferlan wrote:
>>> In preparation for privatizing the virNetworkObj structure, create
>>> accessors for the obj->autostart.
>>>
>>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>>> ---
>>> src/conf/virnetworkobj.c | 15 +++++++++++++++
>>> src/conf/virnetworkobj.h | 9 ++++++++-
>>> src/libvirt_private.syms | 2 ++
>>> src/network/bridge_driver.c | 20 ++++++++++----------
>>> src/test/test_driver.c | 5 +++--
>>> 5 files changed, 38 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
>>> index 631f8cd..36d4bff 100644
>>> --- a/src/conf/virnetworkobj.c
>>> +++ b/src/conf/virnetworkobj.c
>>> @@ -129,6 +129,21 @@ virNetworkObjGetNewDef(virNetworkObjPtr obj)
>>> }
>>>
>>>
>>> +int
>>> +virNetworkObjGetAutostart(virNetworkObjPtr obj)
>>> +{
>>> + return obj->autostart;
>>> +}
>>> +
>>> +
>>> +void
>>> +virNetworkObjSetAutostart(virNetworkObjPtr obj,
>>> + int autostart)
>>> +{
>>> + obj->autostart = autostart;
>>> +}
>>> +
>>> +
>>> pid_t
>>> virNetworkObjGetDnsmasqPid(virNetworkObjPtr obj)
>>> {
>>> diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
>>> index 90ce0ca..a526d30 100644
>>> --- a/src/conf/virnetworkobj.h
>>> +++ b/src/conf/virnetworkobj.h
>>> @@ -32,7 +32,7 @@ struct _virNetworkObj {
>>> pid_t dnsmasqPid;
>>> pid_t radvdPid;
>>> unsigned int active : 1;
>>> - unsigned int autostart : 1;
>>> + int autostart;
>>
>> Since we are touching this does it make sense to convert this to bool?
>> Interestingly, you're doing that conversion for @active in the next patch.
>>
>
> I think because it was an external API provided value I left it at
> 'int'. For the other two active and persistent - those are boolean
> states as a result of other internal operations and not outwardly
> facing. IOW: There's no virNetworkSetPersistent or virNetworkSetActive
> type API's.
>
> I think also the GetAutostart algorithm which assigns *autostart =
> obj->autostart caused me to pause especially since the code is
> "repeated" in domain, storage, and test. Initially I think I was
> thinking of combining all those places that make the same sequence of
> calls, but that got shot down. I think changing it to a bool would
> probably be a "next step" exercise, but "technically", the Get algorithm
> would be *autostart = obj->autostart ? 1 : 0; as opposed to the current
> *autostart = obj->autostart;
I don't think that different get is a problem. But okay, I don't care
that much to stop this. If you want to change it to bool do, if not I
can live with that too.
ACK if you fix the mem leak issue.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
