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;
unsigned int persistent : 1;
virNetworkDefPtr def; /* The current definition */
@@ -60,6 +60,13 @@ virNetworkObjSetDef(virNetworkObjPtr obj,
virNetworkDefPtr
virNetworkObjGetNewDef(virNetworkObjPtr obj);
+int
+virNetworkObjGetAutostart(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetAutostart(virNetworkObjPtr obj,
+ int autostart);
+
virMacMapPtr
virNetworkObjGetMacMap(virNetworkObjPtr obj);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 84e3eb7..8a3284f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -944,6 +944,7 @@ virNetworkObjFindByName;
virNetworkObjFindByNameLocked;
virNetworkObjFindByUUID;
virNetworkObjFindByUUIDLocked;
+virNetworkObjGetAutostart;
virNetworkObjGetClassIdMap;
virNetworkObjGetDef;
virNetworkObjGetDnsmasqPid;
@@ -966,6 +967,7 @@ virNetworkObjNew;
virNetworkObjRemoveInactive;
virNetworkObjReplacePersistentDef;
virNetworkObjSaveStatus;
+virNetworkObjSetAutostart;
virNetworkObjSetDef;
virNetworkObjSetDefTransient;
virNetworkObjSetDnsmasqPid;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b4fbfc5..eb814d3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -525,7 +525,7 @@ networkAutostartConfig(virNetworkObjPtr obj,
int ret = -1;
virObjectLock(obj);
- if (obj->autostart &&
+ if (virNetworkObjGetAutostart(obj) &&
!virNetworkObjIsActive(obj) &&
networkStartNetwork(driver, obj) < 0)
goto cleanup;
@@ -3963,7 +3963,7 @@ networkGetAutostart(virNetworkPtr net,
if (virNetworkGetAutostartEnsureACL(net->conn, virNetworkObjGetDef(obj)) < 0)
goto cleanup;
- *autostart = obj->autostart;
+ *autostart = virNetworkObjGetAutostart(obj);
ret = 0;
cleanup:
@@ -3974,12 +3974,13 @@ networkGetAutostart(virNetworkPtr net,
static int
networkSetAutostart(virNetworkPtr net,
- int autostart)
+ int new_autostart)
{
virNetworkDriverStatePtr driver = networkGetDriver();
virNetworkObjPtr obj;
virNetworkDefPtr def;
char *configFile = NULL, *autostartLink = NULL;
+ int cur_autostart;
int ret = -1;
if (!(obj = networkObjFromNetwork(net)))
@@ -3995,9 +3996,9 @@ networkSetAutostart(virNetworkPtr net,
goto cleanup;
}
- autostart = (autostart != 0);
-
- if (obj->autostart != autostart) {
+ new_autostart = (new_autostart != 0);
+ cur_autostart = virNetworkObjGetAutostart(obj);
+ if (cur_autostart != new_autostart) {
if ((configFile = virNetworkConfigFile(driver->networkConfigDir,
def->name)) == NULL)
goto cleanup;
@@ -4005,7 +4006,7 @@ networkSetAutostart(virNetworkPtr net,
def->name)) == NULL)
goto cleanup;
- if (autostart) {
+ if (new_autostart) {
if (virFileMakePath(driver->networkAutostartDir) < 0) {
virReportSystemError(errno,
_("cannot create autostart directory '%s'"),
@@ -4028,13 +4029,12 @@ networkSetAutostart(virNetworkPtr net,
}
}
- obj->autostart = autostart;
+ virNetworkObjSetAutostart(obj, new_autostart);
}
+
ret = 0;
cleanup:
- VIR_FREE(configFile);
- VIR_FREE(autostartLink);
virNetworkObjEndAPI(&obj);
return ret;
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 868aa27..8e841b2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3647,7 +3647,7 @@ testNetworkGetAutostart(virNetworkPtr net,
if (!(obj = testNetworkObjFindByName(privconn, net->name)))
goto cleanup;
- *autostart = obj->autostart;
+ *autostart = virNetworkObjGetAutostart(obj);
ret = 0;
cleanup:
@@ -3667,7 +3667,8 @@ testNetworkSetAutostart(virNetworkPtr net,
if (!(obj = testNetworkObjFindByName(privconn, net->name)))
goto cleanup;
- obj->autostart = autostart ? 1 : 0;
+ virNetworkObjSetAutostart(obj, autostart ? 1 : 0);
+
ret = 0;
cleanup:
--
2.9.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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. > unsigned int persistent : 1; > > virNetworkDefPtr def; /* The current definition */ > @@ -60,6 +60,13 @@ virNetworkObjSetDef(virNetworkObjPtr obj, > virNetworkDefPtr > virNetworkObjGetNewDef(virNetworkObjPtr obj); > > +int > +virNetworkObjGetAutostart(virNetworkObjPtr obj); > + > +void > +virNetworkObjSetAutostart(virNetworkObjPtr obj, > + int autostart); > + > virMacMapPtr > virNetworkObjGetMacMap(virNetworkObjPtr obj); > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 84e3eb7..8a3284f 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -944,6 +944,7 @@ virNetworkObjFindByName; > virNetworkObjFindByNameLocked; > virNetworkObjFindByUUID; > virNetworkObjFindByUUIDLocked; > +virNetworkObjGetAutostart; > virNetworkObjGetClassIdMap; > virNetworkObjGetDef; > virNetworkObjGetDnsmasqPid; > @@ -966,6 +967,7 @@ virNetworkObjNew; > virNetworkObjRemoveInactive; > virNetworkObjReplacePersistentDef; > virNetworkObjSaveStatus; > +virNetworkObjSetAutostart; > virNetworkObjSetDef; > virNetworkObjSetDefTransient; > virNetworkObjSetDnsmasqPid; > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index b4fbfc5..eb814d3 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -525,7 +525,7 @@ networkAutostartConfig(virNetworkObjPtr obj, > int ret = -1; > > virObjectLock(obj); > - if (obj->autostart && > + if (virNetworkObjGetAutostart(obj) && > !virNetworkObjIsActive(obj) && > networkStartNetwork(driver, obj) < 0) > goto cleanup; > @@ -3963,7 +3963,7 @@ networkGetAutostart(virNetworkPtr net, > if (virNetworkGetAutostartEnsureACL(net->conn, virNetworkObjGetDef(obj)) < 0) > goto cleanup; > > - *autostart = obj->autostart; > + *autostart = virNetworkObjGetAutostart(obj); > ret = 0; > > cleanup: > @@ -3974,12 +3974,13 @@ networkGetAutostart(virNetworkPtr net, > > static int > networkSetAutostart(virNetworkPtr net, > - int autostart) > + int new_autostart) > { > virNetworkDriverStatePtr driver = networkGetDriver(); > virNetworkObjPtr obj; > virNetworkDefPtr def; > char *configFile = NULL, *autostartLink = NULL; > + int cur_autostart; > int ret = -1; > > if (!(obj = networkObjFromNetwork(net))) > @@ -3995,9 +3996,9 @@ networkSetAutostart(virNetworkPtr net, > goto cleanup; > } > > - autostart = (autostart != 0); > - > - if (obj->autostart != autostart) { > + new_autostart = (new_autostart != 0); > + cur_autostart = virNetworkObjGetAutostart(obj); > + if (cur_autostart != new_autostart) { > if ((configFile = virNetworkConfigFile(driver->networkConfigDir, > def->name)) == NULL) > goto cleanup; > @@ -4005,7 +4006,7 @@ networkSetAutostart(virNetworkPtr net, > def->name)) == NULL) > goto cleanup; > > - if (autostart) { > + if (new_autostart) { > if (virFileMakePath(driver->networkAutostartDir) < 0) { > virReportSystemError(errno, > _("cannot create autostart directory '%s'"), > @@ -4028,13 +4029,12 @@ networkSetAutostart(virNetworkPtr net, > } > } > > - obj->autostart = autostart; > + virNetworkObjSetAutostart(obj, new_autostart); > } > + > ret = 0; > > cleanup: > - VIR_FREE(configFile); > - VIR_FREE(autostartLink); This doesn't feel right. @configFile and @autostartLink are leaked. > virNetworkObjEndAPI(&obj); > return ret; > } Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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; >> unsigned int persistent : 1; >> >> virNetworkDefPtr def; /* The current definition */ >> @@ -60,6 +60,13 @@ virNetworkObjSetDef(virNetworkObjPtr obj, >> virNetworkDefPtr >> virNetworkObjGetNewDef(virNetworkObjPtr obj); >> >> +int >> +virNetworkObjGetAutostart(virNetworkObjPtr obj); >> + >> +void >> +virNetworkObjSetAutostart(virNetworkObjPtr obj, >> + int autostart); >> + >> virMacMapPtr >> virNetworkObjGetMacMap(virNetworkObjPtr obj); >> >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index 84e3eb7..8a3284f 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -944,6 +944,7 @@ virNetworkObjFindByName; >> virNetworkObjFindByNameLocked; >> virNetworkObjFindByUUID; >> virNetworkObjFindByUUIDLocked; >> +virNetworkObjGetAutostart; >> virNetworkObjGetClassIdMap; >> virNetworkObjGetDef; >> virNetworkObjGetDnsmasqPid; >> @@ -966,6 +967,7 @@ virNetworkObjNew; >> virNetworkObjRemoveInactive; >> virNetworkObjReplacePersistentDef; >> virNetworkObjSaveStatus; >> +virNetworkObjSetAutostart; >> virNetworkObjSetDef; >> virNetworkObjSetDefTransient; >> virNetworkObjSetDnsmasqPid; >> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c >> index b4fbfc5..eb814d3 100644 >> --- a/src/network/bridge_driver.c >> +++ b/src/network/bridge_driver.c >> @@ -525,7 +525,7 @@ networkAutostartConfig(virNetworkObjPtr obj, >> int ret = -1; >> >> virObjectLock(obj); >> - if (obj->autostart && >> + if (virNetworkObjGetAutostart(obj) && >> !virNetworkObjIsActive(obj) && >> networkStartNetwork(driver, obj) < 0) >> goto cleanup; >> @@ -3963,7 +3963,7 @@ networkGetAutostart(virNetworkPtr net, >> if (virNetworkGetAutostartEnsureACL(net->conn, virNetworkObjGetDef(obj)) < 0) >> goto cleanup; >> >> - *autostart = obj->autostart; >> + *autostart = virNetworkObjGetAutostart(obj); >> ret = 0; >> >> cleanup: >> @@ -3974,12 +3974,13 @@ networkGetAutostart(virNetworkPtr net, >> >> static int >> networkSetAutostart(virNetworkPtr net, >> - int autostart) >> + int new_autostart) >> { >> virNetworkDriverStatePtr driver = networkGetDriver(); >> virNetworkObjPtr obj; >> virNetworkDefPtr def; >> char *configFile = NULL, *autostartLink = NULL; >> + int cur_autostart; >> int ret = -1; >> >> if (!(obj = networkObjFromNetwork(net))) >> @@ -3995,9 +3996,9 @@ networkSetAutostart(virNetworkPtr net, >> goto cleanup; >> } >> >> - autostart = (autostart != 0); >> - >> - if (obj->autostart != autostart) { >> + new_autostart = (new_autostart != 0); >> + cur_autostart = virNetworkObjGetAutostart(obj); >> + if (cur_autostart != new_autostart) { >> if ((configFile = virNetworkConfigFile(driver->networkConfigDir, >> def->name)) == NULL) >> goto cleanup; >> @@ -4005,7 +4006,7 @@ networkSetAutostart(virNetworkPtr net, >> def->name)) == NULL) >> goto cleanup; >> >> - if (autostart) { >> + if (new_autostart) { >> if (virFileMakePath(driver->networkAutostartDir) < 0) { >> virReportSystemError(errno, >> _("cannot create autostart directory '%s'"), >> @@ -4028,13 +4029,12 @@ networkSetAutostart(virNetworkPtr net, >> } >> } >> >> - obj->autostart = autostart; >> + virNetworkObjSetAutostart(obj, new_autostart); >> } >> + >> ret = 0; >> >> cleanup: >> - VIR_FREE(configFile); >> - VIR_FREE(autostartLink); > > This doesn't feel right. @configFile and @autostartLink are leaked. > oh right - I dragged back the bulk of the algorithm, but not the VIR_FREE's... I'll restore them. Thanks - John >> virNetworkObjEndAPI(&obj); >> return ret; >> } > > Michal > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[...] >> >> 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 > Alright - I changed to @bool... I also changed GetAutostart to be IsAutostart, but otherwise was (mostly) simple except for merges in the subsequent 2 patches. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.