On Thu, May 27, 2010 at 02:46:33PM -0400, Cole Robinson wrote:
> On 05/27/2010 12:00 PM, Daniel P. Berrange wrote:
> > The network driver is not doing correct checking for
> > duplicate UUID/name values. This introduces a new method
> > virNetworkObjIsDuplicate, based on the previously
> > written virDomainObjIsDuplicate.
> >
> > * src/conf/network_conf.c, src/conf/network_conf.c,
> > src/libvirt_private.syms: Add virNetworkObjIsDuplicate,
> > * src/network/bridge_driver.c: Call virNetworkObjIsDuplicate
> > for checking uniqueness of uuid/names
> > ---
> > src/conf/network_conf.c | 65 +++++++++++++++++++++++++++++++++++++++++++
> > src/conf/network_conf.h | 4 ++
> > src/libvirt_private.syms | 1 +
> > src/network/bridge_driver.c | 7 ++++
> > 4 files changed, 77 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> > index 06537d1..347fc0b 100644
> > --- a/src/conf/network_conf.c
> > +++ b/src/conf/network_conf.c
> > @@ -940,6 +940,71 @@ error:
> > return ret;
> > }
> >
> > +
> > +/*
> > + * virNetworkObjIsDuplicate:
> > + * @doms : virNetworkObjListPtr to search
> > + * @def : virNetworkDefPtr definition of network to lookup
> > + * @check_active: If true, ensure that network is not active
> > + *
> > + * Returns: -1 on error
> > + * 0 if network is new
> > + * 1 if network is a duplicate
> > + */
> > +int
> > +virNetworkObjIsDuplicate(virNetworkObjListPtr doms,
> > + virNetworkDefPtr def,
> > + unsigned int check_active)
> > +{
> > + int ret = -1;
> > + int dupVM = 0;
> > + virNetworkObjPtr vm = NULL;
> > +
> > + /* See if a VM with matching UUID already exists */
> > + vm = virNetworkFindByUUID(doms, def->uuid);
> > + if (vm) {
> > + /* UUID matches, but if names don't match, refuse it */
> > + if (STRNEQ(vm->def->name, def->name)) {
> > + char uuidstr[VIR_UUID_STRING_BUFLEN];
> > + virUUIDFormat(vm->def->uuid, uuidstr);
> > + virNetworkReportError(VIR_ERR_OPERATION_FAILED,
> > + _("network '%s' is already defined with uuid %s"),
> > + vm->def->name, uuidstr);
> > + goto cleanup;
> > + }
> > +
> > + if (check_active) {
> > + /* UUID & name match, but if VM is already active, refuse it */
> > + if (virNetworkObjIsActive(vm)) {
> > + virNetworkReportError(VIR_ERR_OPERATION_INVALID,
> > + _("network is already active as '%s'"),
> > + vm->def->name);
> > + goto cleanup;
> > + }
> > + }
> > +
> > + dupVM = 1;
> > + } else {
> > + /* UUID does not match, but if a name matches, refuse it */
> > + vm = virNetworkFindByName(doms, def->name);
> > + if (vm) {
> > + char uuidstr[VIR_UUID_STRING_BUFLEN];
> > + virUUIDFormat(vm->def->uuid, uuidstr);
> > + virNetworkReportError(VIR_ERR_OPERATION_FAILED,
> > + _("network '%s' already exists with uuid %s"),
> > + def->name, uuidstr);
> > + goto cleanup;
> > + }
> > + }
> > +
> > + ret = dupVM;
> > +cleanup:
> > + if (vm)
> > + virNetworkObjUnlock(vm);
> > + return ret;
> > +}
> > +
> > +
> > void virNetworkObjLock(virNetworkObjPtr obj)
> > {
> > virMutexLock(&obj->lock);
> > diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> > index 127a23a..c4956b6 100644
> > --- a/src/conf/network_conf.h
> > +++ b/src/conf/network_conf.h
> > @@ -169,6 +169,10 @@ int virNetworkSetBridgeName(const virNetworkObjListPtr nets,
> > virNetworkDefPtr def,
> > int check_collision);
> >
> > +int virNetworkObjIsDuplicate(virNetworkObjListPtr doms,
> > + virNetworkDefPtr def,
> > + unsigned int check_active);
> > +
> > void virNetworkObjLock(virNetworkObjPtr obj);
> > void virNetworkObjUnlock(virNetworkObjPtr obj);
> >
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index e7cd6dd..414e937 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -469,6 +469,7 @@ virNetworkSaveConfig;
> > virNetworkSetBridgeName;
> > virNetworkObjLock;
> > virNetworkObjUnlock;
> > +virNetworkObjIsDuplicate;
> >
> >
> > # nodeinfo.h
> > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> > index 5d7ef19..01be425 100644
> > --- a/src/network/bridge_driver.c
> > +++ b/src/network/bridge_driver.c
> > @@ -1265,6 +1265,9 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) {
> > if (!(def = virNetworkDefParseString(xml)))
> > goto cleanup;
> >
> > + if (virNetworkObjIsDuplicate(&driver->networks, def, 1) < 0)
> > + goto cleanup;
> > +
> > if (virNetworkSetBridgeName(&driver->networks, def, 1))
> > goto cleanup;
> >
> > @@ -1295,12 +1298,16 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
> > virNetworkDefPtr def;
> > virNetworkObjPtr network = NULL;
> > virNetworkPtr ret = NULL;
> > + int dupNet;
> >
>
> Why not just check the error code directly like in the other use?
>
> Aside from that, ACK
I was just copying the code from the QEMU driver, which uses this variable
to emit events a short while later. We need to make the network driver
emit events too.....
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list