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@redhat.com> --- 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