On 2012年08月20日 14:17, Laine Stump wrote:

We need to be able to pass a flag when (re)defining a network that says to enact the changes immediately rather than waiting until the next restart of the network, but the existing virNetworkDefineXML has no flags arg.

This patch adds a new public API virNetworkDefineXMLFlags that will be identical to virNetworkDefineXML, but with an added flags arg (which initially will only accept "0" on the remote end). --- include/libvirt/libvirt.h.in | 11 ++++++++++ src/driver.h | 5 +++++ src/libvirt.c | 49 +++++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 1 + 4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 91e0a29..70bb594 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2266,6 +2266,17 @@ virNetworkPtr virNetworkLookupByUUIDString (virConnectPtr conn, virNetworkPtr virNetworkCreateXML (virConnectPtr conn, const char *xmlDesc);

+typedef enum { + VIR_NETWORK_DEFINE_NONE = 0, /* default behavior */ +} virNetworkDefineFlags; + +/* + * Define inactive persistent network with flags + */ +virNetworkPtr virNetworkDefineXMLFlags(virConnectPtr conn, + const char *xmlDesc, + unsigned int flags); + /* * Define inactive persistent network */ diff --git a/src/driver.h b/src/driver.h index aab9766..b25a6ac 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1072,6 +1072,10 @@ typedef virNetworkPtr (*virDrvNetworkCreateXML) (virConnectPtr conn, const char *xmlDesc); typedef virNetworkPtr + (*virDrvNetworkDefineXMLFlags) (virConnectPtr conn, + const char *xml, + unsigned int flags); +typedef virNetworkPtr (*virDrvNetworkDefineXML) (virConnectPtr conn, const char *xml); typedef int @@ -1123,6 +1127,7 @@ struct _virNetworkDriver { virDrvNetworkLookupByUUID networkLookupByUUID; virDrvNetworkLookupByName networkLookupByName; virDrvNetworkCreateXML networkCreateXML; + virDrvNetworkDefineXMLFlags networkDefineXMLFlags; virDrvNetworkDefineXML networkDefineXML; virDrvNetworkUndefine networkUndefine; virDrvNetworkCreate networkCreate; diff --git a/src/libvirt.c b/src/libvirt.c index 893d380..aa9f101 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9920,11 +9920,58 @@ error: }

/** + * virNetworkDefineXMLFlags: + * @conn: pointer to the hypervisor connection + * @xml: the XML description for the network, preferably in UTF-8 + * @flags: bitwise or of virNetworkDefineFlags (currently must be 0). + * + * Define a network, but do not create it. This function is + * identical to the older virNetworkDefineXML, but with the addition + * of the flags argument. + * + * Returns NULL in case of error, a pointer to the network otherwise + */ +virNetworkPtr +virNetworkDefineXMLFlags(virConnectPtr conn, const char *xml, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, xml=%s, flags=0x%x", conn, xml, flags);

Indention problem.

+ + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + if (conn->flags& VIR_CONNECT_RO) { + virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + virCheckNonNullArgGoto(xml, error); + + if (conn->networkDriver&& conn->networkDriver->networkDefineXMLFlags) { + virNetworkPtr ret; + ret = conn->networkDriver->networkDefineXMLFlags(conn, xml, flags); + if (!ret) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return NULL; +} + +/** * virNetworkDefineXML: * @conn: pointer to the hypervisor connection * @xml: the XML description for the network, preferably in UTF-8 * - * Define a network, but does not create it + * Define a network, but do not create it. See + * virNetworkDefineXMLFlags() for more control. * * Returns NULL in case of error, a pointer to the network otherwise */ diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index e3ba119..c814cb7 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -549,6 +549,7 @@ LIBVIRT_0.10.0 { virDomainGetHostname; virConnectRegisterCloseCallback; virConnectUnregisterCloseCallback; + virNetworkDefineXMLFlags;

Conflicts with the atomic APIs, I can rebase if you push first. :-) ACK.

On 08/20/2012 12:17 AM, Laine Stump wrote:

Just add an entry to the function table, rename the original virNetworkDefineXML to virNetworkDefineXMLFlags (with a check for flags == 0), and add a one line replacement for virNetworkDefineXML that calls virNetworkDefineXMLFlags. --- src/network/bridge_driver.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index bff2d30..e2b569f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2339,7 +2339,10 @@ cleanup: return ret; }

-static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { +static +virNetworkPtr networkDefineXMLFlags(virConnectPtr conn, const char *xml, + unsigned int flags)

Unusual formatting. When using split line declarations, our style is: static virNetworkPtr networkDefineXMLFlags(...) That is, the function name always starts in column 1 in split style. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 2012年08月20日 14:17, Laine Stump wrote:

Just add an entry to the function table, rename the original virNetworkDefineXML to virNetworkDefineXMLFlags (with a check for flags == 0), and add a one line replacement for virNetworkDefineXML. --- src/test/test_driver.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a767e21..e0da9fe 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3105,12 +3105,17 @@ cleanup: return ret; }

-static virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) { +static +virNetworkPtr testNetworkDefineXMLFlags(virConnectPtr conn, const char *xml, + unsigned int flags) +{ testConnPtr privconn = conn->privateData; virNetworkDefPtr def; virNetworkObjPtr net = NULL; virNetworkPtr ret = NULL;

+ virCheckFlags(0, NULL); + testDriverLock(privconn); if ((def = virNetworkDefParseString(xml)) == NULL) goto cleanup; @@ -3130,6 +3135,12 @@ cleanup: return ret; }

+static +virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) +{ + return testNetworkDefineXMLFlags(conn, xml, 0); +} + static int testNetworkUndefine(virNetworkPtr network) { testConnPtr privconn = network->conn->privateData; virNetworkObjPtr privnet; @@ -5616,6 +5627,7 @@ static virNetworkDriver testNetworkDriver = { .networkLookupByUUID = testLookupNetworkByUUID, /* 0.3.2 */ .networkLookupByName = testLookupNetworkByName, /* 0.3.2 */ .networkCreateXML = testNetworkCreate, /* 0.3.2 */ + .networkDefineXMLFlags = testNetworkDefineXMLFlags, /* 0.10.0 */ .networkDefineXML = testNetworkDefine, /* 0.3.2 */ .networkUndefine = testNetworkUndefine, /* 0.3.2 */ .networkCreate = testNetworkStart, /* 0.3.2 */

ACK

On 2012年08月20日 14:17, Laine Stump wrote:

Currently there is no practical difference between this and using the old virNetworkDefinXML API, it just allows using the new API to make sure it's working.

Soon there will be a flag defined for the new function (to request changes in re-definitions of existing+active networks take effect immediately), and virsh will add support for that flag. --- tools/virsh-network.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 49ec34f..414ca86 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -181,6 +181,7 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; bool ret = true; char *buffer; + unsigned flags = 0;

if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -191,7 +192,16 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VIRSH_MAX_XML_FILE,&buffer)< 0) return false;

- network = virNetworkDefineXML(ctl->conn, buffer); + network = virNetworkDefineXMLFlags(ctl->conn, buffer, flags); + + if (network == NULL&& last_error->code == VIR_ERR_NO_SUPPORT) {

It should reset the error before trying the old API: vshResetLibvirtError();

+ /* fall back to older function that doesn't take flags. note + * that if we want to use flags in the future, we'll have to + * error out here when flags != 0. + */ + network = virNetworkDefineXML(ctl->conn, buffer); + } + VIR_FREE(buffer);

if (network != NULL) {

ACK with the nit fixed (with wondering why not use new API in virsh when the flags is/are supported though). Regards, Osier