[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv4 4/5] domifaddr: Add virsh support



On Tue, Aug 27, 2013 at 07:28:23PM +0530, Nehal J Wani wrote:
> On Tue, Aug 27, 2013 at 6:20 PM, Daniel P. Berrange <berrange redhat com>
> wrote:
> >
> > On Sun, Aug 25, 2013 at 04:45:44AM +0530, Nehal J Wani wrote:
> > > Use virDomainInterfacesAddresses in virsh
> > >
> > > tools/virsh-domain-monitor.c
> > >    * Introduce new command : domifaddr
> > >    virsh # domifaddr f18
> > >    Name       MAC address          IPv4 address        IPv6 address
> > >
>  -------------------------------------------------------------------------------
> > >    lo         00:00:00:00:00:00    127.0.0.1/8
>  ::1/128
> > >    eth0       52:54:00:89:4e:97    192.168.101.130/24 fe80::5054:ff:fe89:4e97/64
> > >    eth0:1     52:54:00:89:4e:97    192.168.101.133/24
> > >    eth0:2     52:54:00:89:4e:97    192.168.101.132/24
> > >    eth1       52:54:00:89:ad:35    192.168.102.142/24 fe80::5054:ff:fe89:ad35/64
> > >    eth1:1     52:54:00:89:ad:35    192.168.102.143/24
> > >    eth2       52:54:00:d3:39:ee    192.168.103.183/24 fe80::5054:ff:fed3:39ee/64
> > >    eth2:0     52:54:00:d3:39:ee    192.168.103.184/24
> > >    eth2:1     52:54:00:d3:39:ee    192.168.103.185/24
> > >    eth3       52:54:00:fe:4c:4f    192.168.101.197/24 fe80::5054:ff:fefe:4c4f/64
> > >    eth3:1     52:54:00:fe:4c:4f    192.168.101.198/24
> >
> > This formatting of IP addrs is broken.
> >
> > We should not expose interface aliases 'eth0:1', 'eth0:2', etc. If QEMU
> agent
> > is returning such names, either we should fix the agent, or strip the ":1"
> > suffixes in libvirt. The aliased names are an artifact of the legacy
> linux IP
> > config tools. The new 'ip' command does not use these - it just shows
> 'eth0'
> > with multiple IPv4 and multiple IPv6 addresses, which is also how
> libvirt/netcf
> > report physical device names & config.
> >
> > Our display format must allow for NICs having arbitrarily many addresses
> > of either type, so displaying IPv4/IPv6 side by side will not work.
> >
> > I think we need a display format like:
> >
> >   virsh  domifaddr f18
> >     Name       MAC address          Protocol  Address
> >
> -------------------------------------------------------------------------------
> >     lo         00:00:00:00:00:00    ipv4      127.0.0.1/8
> >     -          -                    ipv6      ::1/128
> >     eth0       52:54:00:89:4e:97    ipv4      192.168.101.130/24
> >     -          -                    ipv4      192.168.101.133/24
> >     -          -                    ipv4      192.168.101.132/24
> >     -          -                    ipv6      fe80::5054:ff:fe89:4e97/64
> >     eth1       52:54:00:89:ad:35    ipv4      192.168.102.142/24
> >     -          -                    ipv4      192.168.102.143/24
> >     -          -                    ipv6      fe80::5054:ff:fe89:ad35/64
> >
> >
> > With option to fully display all fields to make life easier for scripts:
> >
> >   virsh domifaddr --full f18
> >     Name       MAC address          Protocol  Address
> >
> -------------------------------------------------------------------------------
> >     lo         00:00:00:00:00:00    ipv4      127.0.0.1/8
> >     lo         00:00:00:00:00:00    ipv6      ::1/128
> >     eth0       52:54:00:89:4e:97    ipv4      192.168.101.130/24
> >     eth0       52:54:00:89:4e:97    ipv4      192.168.101.133/24
> >     eth0       52:54:00:89:4e:97    ipv4      192.168.101.132/24
> >     eth0       52:54:00:89:4e:97    ipv6      fe80::5054:ff:fe89:4e97/64
> >     eth1       52:54:00:89:ad:35    ipv4      192.168.102.142/24
> >     eth1       52:54:00:89:ad:35    ipv4      192.168.102.143/24
> >     eth1       52:54:00:89:ad:35    ipv6      fe80::5054:ff:fe89:ad35/64
> >
> >
> > > +
> > > +    for (i = 0; i < ifaces_count; i++) {
> > > +        virDomainInterfacePtr iface = ifaces[i];
> > > +        virBuffer buf = VIR_BUFFER_INITIALIZER;
> > > +        const char *hwaddr = "";
> > > +        const char *ip_addr_str = NULL;
> > > +
> > > +        if (interface && STRNEQ(interface, iface->name)) {
> > > +            virBufferFreeAndReset(&buf);
> > > +            continue;
> > > +        }
> > > +
> > > +        if (iface->hwaddr)
> > > +            hwaddr = iface->hwaddr;
> > > +
> > > +        for (j = 0; j < iface->naddrs; j++) {
> > > +            if (j)
> > > +                virBufferAsprintf(&buf, "%25s/%d",
> > > +                                  iface->addrs[j].addr,
> > > +                                  iface->addrs[j].prefix);
> > > +            else
> > > +                virBufferAsprintf(&buf, "%s/%d",
> > > +                                  iface->addrs[j].addr,
> > > +                                  iface->addrs[j].prefix);
> >
> > This logic is very broken not allowing for multiple addrs per device
> >
> > > +        }
> > > +
> > > +        if (virBufferError(&buf)) {
> > > +            virBufferFreeAndReset(&buf);
> > > +            virReportOOMError();
> > > +            return ret;
> > > +        }
> > > +
> > > +        ip_addr_str = virBufferContentAndReset(&buf);
> > > +
> > > +        if (!ip_addr_str)
> > > +            ip_addr_str = "";
> > > +
> > > +        vshPrintExtra(ctl, " %-10s %-17s    %s\n",
> > > +                     iface->name, hwaddr, ip_addr_str);
> > > +
> > > +        virBufferFreeAndReset(&buf);
> > > +    }
> > > +
> > > +    ret = true;
> > > +
> > > +cleanup:
> > > +    for (i = 0; i < ifaces_count; i++)
> > > +        virDomainInterfaceFree(ifaces[i]);
> > > +    VIR_FREE(ifaces);
> > > +
> > > +    virDomainFree(dom);
> > > +    return ret;
> > > +}
> > > +
> > >  const vshCmdDef domMonitoringCmds[] = {
> > >      {.name = "domblkerror",
> > >       .handler = cmdDomBlkError,
> > > @@ -1944,5 +2039,11 @@ const vshCmdDef domMonitoringCmds[] = {
> > >       .info = info_list,
> > >       .flags = 0
> > >      },
> > > +    {.name = "domifaddr",
> > > +     .handler = cmdDomIfAddr,
> > > +     .opts = opts_domifaddr,
> > > +     .info = info_domifaddr,
> > > +     .flags = 0
> > > +    },
> > >      {.name = NULL}
> > >  };
> >
> >
> > Daniel
> > --
> > |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/:|
> > |: http://libvirt.org              -o-             http://virt-manager.org:|
> > |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/:|
> > |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc:|
> >
> > --
> > libvir-list mailing list
> > libvir-list redhat com
> > https://www.redhat.com/mailman/listinfo/libvir-list
> 
> Suppose I have the following network configuration in my guest:
> [root localhost ~]# ifconfig
> eth0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet 192.168.154.8  netmask 255.255.0.0  broadcast 192.168.255.255
>         inet6 fe80::5054:ff:fefe:4c4f  prefixlen 64  scopeid 0x20<link>
>         inet6 2001:db8:0:f101::2  prefixlen 64  scopeid 0x0<global>
>         inet6 2001:db8:0:f101::1  prefixlen 64  scopeid 0x0<global>
>         ether 52:54:00:fe:4c:4f  txqueuelen 1000  (Ethernet)
>         RX packets 1535  bytes 123240 (120.3 KiB)
>         RX errors 0  dropped 0  overruns 0  frame 0
>         TX packets 1133  bytes 160636 (156.8 KiB)
>         TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
> 
> eth0:0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet 102.168.168.168  netmask 255.255.0.0  broadcast 192.168.255.255
>         ether 52:54:00:fe:4c:4f  txqueuelen 1000  (Ethernet)
> 
> eth0:1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet 192.168.101.197  netmask 255.255.255.0  broadcast
> 192.168.101.255
>         ether 52:54:00:fe:4c:4f  txqueuelen 1000  (Ethernet)
> 
> lo: flags=73<UP,LOOPBACK,RUNNING>  mtu 16436
>         inet 127.0.0.1  netmask 255.0.0.0
>         inet6 ::1  prefixlen 128  scopeid 0x10<host>
>         loop  txqueuelen 0  (Local Loopback)
>         RX packets 8  bytes 616 (616.0 B)
>         RX errors 0  dropped 0  overruns 0  frame 0
>         TX packets 8  bytes 616 (616.0 B)
>         TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
> 
> [root localhost ~]# ip addr
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
>     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>     inet 127.0.0.1/8 scope host lo
>     inet6 ::1/128 scope host
>        valid_lft forever preferred_lft forever
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state
> UP qlen 1000
>     link/ether 52:54:00:fe:4c:4f brd ff:ff:ff:ff:ff:ff
>     inet 192.168.154.8/16 brd 192.168.255.255 scope global eth0
>     inet 192.168.101.197/24 brd 192.168.101.255 scope global eth0:1
>     inet 102.168.168.168/16 brd 192.168.255.255 scope global eth0:0
>     inet6 2001:db8:0:f101::2/64 scope global
>        valid_lft forever preferred_lft forever
>     inet6 2001:db8:0:f101::1/64 scope global
>        valid_lft forever preferred_lft forever
>     inet6 fe80::5054:ff:fefe:4c4f/64 scope link
>        valid_lft forever preferred_lft forever
> [root localhost ~]#
> 
> Now, qemu-guest-agent returns back (after making it pretty):
> {
> 
>   "return": [
>     {
>       "name": "lo",
>       "ip-addresses": [
>         {
>           "ip-address-type": "ipv4",
>           "ip-address": "127.0.0.1",
>           "prefix": 8
>         },
>         {
>           "ip-address-type": "ipv6",
>           "ip-address": "::1",
>           "prefix": 128
>         }
>       ],
>       "hardware-address": "00:00:00:00:00:00"
>     },
>     {
>       "name": "eth0",
>       "ip-addresses": [
>         {
>           "ip-address-type": "ipv4",
>           "ip-address": "192.168.154.8",
>           "prefix": 16
>         },
>         {
>           "ip-address-type": "ipv6",
>           "ip-address": "2001:db8:0:f101::2",
>           "prefix": 64
>         },
>         {
>           "ip-address-type": "ipv6",
>           "ip-address": "2001:db8:0:f101::1",
>           "prefix": 64
>         },
>         {
>           "ip-address-type": "ipv6",
>           "ip-address": "fe80::5054:ff:fefe:4c4f",
>           "prefix": 64
>         }
>       ],
>       "hardware-address": "52:54:00:fe:4c:4f"
>     },
>     {
>       "name": "eth0:1",
>       "ip-addresses": [
>         {
>           "ip-address-type": "ipv4",
>           "ip-address": "192.168.101.197",
>           "prefix": 24
>         }
>       ],
>       "hardware-address": "52:54:00:fe:4c:4f"
>     },
>     {
>       "name": "eth0:0",
>       "ip-addresses": [
>         {
>           "ip-address-type": "ipv4",
>           "ip-address": "102.168.168.168",
>           "prefix": 16
>         }
>       ],
>       "hardware-address": "52:54:00:fe:4c:4f"
>     }
>   ]
> }
> 
> So, qemu-ga doesn't understand that there can't be more than one
> device with same MAC addr. So, I think we are left with the following
> options:

Actually that's wrong. You *can* have 2 completely different physical
NICs with the same MAC address.

> (i) Modify qemu-guest-agent to return addresses belonging to same
> MAC address grouped under one interface only.

You would not want to group based on MAC address - you explicitly just
want to normalize by stripping the legacy aliases suffixes.

> OR
> (ii) Let the reply be as it is now. Strip the ":0", ":1" from the response
> of guest agent (Is this really necessary?) . We'll have to parse the JSON
> multiple times and fill the virDomainInterface structs by grouping them
> according to the MAC addresses.

I think we need to do (ii) regardless to cope with existing deployed
QEMU agent versions.

We should also recommend to QEMU developers to fix the agent to not expose
these legacy device alias names.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]