Re: [libvirt] [RESEND][PATCH v2] qemu allow persistent modifications of NICs

On Mon, May 23, 2011 at 11:09:21AM +0900, KAMEZAWA Hiroyuki wrote: > > Slightly updated against the latest git tree. but didn't update version number. > == > Now, only attach/detatch/update for disks are supporeted for inactive > domains. > This patch allows to modify network interfaces of inactive domains. > Users can modify inactive domains with --persistent flag. > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; > > * src/conf/domain_conf.c: > (virDomainNetInsert) : Insert a network device to domain definition. > (virDomainNetIndexByMac) : Returns an index of net device in array. > (virDomainNetRemoveByMac): Remove a NIC of passed MAC address. > > * src/qemu/qemu_driver.c > (qemuDomainAttachDeviceConfig): add codes for NIC. > (qemuDomainDetachDeviceConfig): add codes for NIC. > > Changelog v1->v2: > - fixed virDomainNetInsert > - removed target name check. (It seems I misunderstood something.) > - fixed error check in qemuDomainAttachDeviceConfig > - fixed error message in qemuDomainDetachDeviceConfig > --- > src/conf/domain_conf.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > src/conf/domain_conf.h | 4 ++++ > src/libvirt_private.syms | 3 +++ > src/qemu/qemu_driver.c | 31 +++++++++++++++++++++++++++++++ > 4 files changed, 83 insertions(+), 0 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 6129bbc..d85c8bc 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -5177,6 +5177,51 @@ int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) > return 0; > } > > +int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net) > +{ > + if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) > + return -1; > + def->nets[def->nnets] = net; > + def->nnets++; > + return 0; > +} > + > +int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac) > +{ > + int i; > + > + for (i = 0; i < def->nnets; i++) > + if (!memcmp(def->nets[i]->mac, mac, VIR_MAC_BUFLEN)) > + return i; > + return -1; > +} > + > +static void virDomainNetRemove(virDomainDefPtr def, size_t i) > +{ If i exceeds def->nnets, you may access beyond def->nets, which may corrupt memory.

> + if (def->nnets > 1) { > + memmove(def->nets + i, > + def->nets + i + 1, > + sizeof(*def->nets) * (def->nnets - (i + 1))); > + def->nnets--; > + if (VIR_REALLOC_N(def->nets, def->nnets) < 0) { > + /* ignore harmless */ > + } ignore_value() is provided to ignore a return value.

> + } else { > + VIR_FREE(def->nets); > + def->nnets = 0; > + } > +} > + > +int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac) > +{ > + int i = virDomainNetIndexByMac(def, mac); > + > + if (i < 0) > + return -1; > + virDomainNetRemove(def, i); > + return 0; > +} > + > > int virDomainControllerInsert(virDomainDefPtr def, > virDomainControllerDefPtr controller) > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 5fe31d4..f195caa 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1360,6 +1360,10 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); > void virDomainDiskRemove(virDomainDefPtr def, size_t i); > int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name); > > +int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac); > +int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net); Will it be better to add virDomainNetRemove here as a counterpart of virDomainNetInsert? Although no external file uses it.