Cole Robinson wrote: ... > Okay, I rolled these changes and the BridgeInUse changes into one patch > (along with Jim's suggestions). > > I added a helper function SetBridgeName which basically does: > > if user passed bridge name: > verify it doesn't collide > else: > generate bridge name > > We call this in Define and CreateXML. When loading configs from a driver > restart, we only attempt to generate a bridge: if checking for > collisions failed, the network wouldn't even be defined, which would > only cause more pain for users. ... Hi Cole, Here's a quick review: One nit below, but what I'd really like is a stand-alone virsh-oriented way to exercise a few of these new code paths (tests), just to make sure we get coverage when running "make check". If you can outline something like that, I'll massage it into a new test. > Generate network bridge names if none passed at define/create time. ... > diff --git a/src/network_conf.c b/src/network_conf.c > index 6ad0d01..5de164e 100644 > --- a/src/network_conf.c > +++ b/src/network_conf.c > @@ -43,6 +43,7 @@ > #include "buf.h" > #include "c-ctype.h" > > +#define MAX_BRIDGE_ID 256 ... > +char *virNetworkAllocateBridge(virConnectPtr conn, > + const virNetworkObjListPtr nets) > +{ > + > + int id = 0; > + char *newname; > + > + do { > + char try[50]; > + > + snprintf(try, sizeof(try), "virbr%d", id); > + > + if (!virNetworkBridgeInUse(nets, try, NULL)) { > + if (!(newname = strdup(try))) { > + virReportOOMError(conn); > + return NULL; > + } > + return newname; > + } > + > + id++; > + } while (id < MAX_BRIDGE_ID); This should probably be <= MAX_BRIDGE_ID, or change MAX_BRIDGE_ID to 255, .. so that the diagnostic below makes sense. > + virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, > + _("Bridge generation exceeded max id %d"), > + MAX_BRIDGE_ID); > + return NULL; > +} -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list