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

[KAMEZAWA Hiroyuki]

On Tue, 24 May 2011 09:48:40 +0800
Hu Tao <hutao at cn.fujitsu.com> wrote:

> 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 at jp.fujitsu.com>
> > 
> > * 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.
> 
Hmm, ok, add sanity check.


> > +    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.
> 
will use it.


> > +    } 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.
> 

I don't want to add unused function.  If ByMac annotation is unnecessary,
adding virDomainNetRemove() which replaces virDomainNetRemoveByMac()...

Thanks,
-Kame