Re: [libvirt] [PATCH 4/5] conf: Allow usernet to have an address

2017-09-13 Thread Michal Privoznik
On 09/12/2017 07:35 PM, Laine Stump wrote:
> On 09/12/2017 05:53 AM, Martin Kletzander wrote:
>> On Tue, Sep 12, 2017 at 11:32:52AM +0200, Michal Privoznik wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1075520
>>>
>>> Currently, all that users can specify for an interface type of
>>> 'user' is the common attributes: PCI address, NIC model (and
>>> that's basically it). However, some need to configure other
>>> address range than the default one.
>>>
>>> Signed-off-by: Michal Privoznik 
>>> ---
>>>
>>> Notes:
>>>    Frankly, I'm not that convinced about this one. I mean,
>>>    this puts IP addresses into net->hostIP while we are
>>>    configuring guest side of the SLIRP. However, it just
>>>    feels better to have the IP addresses under 
>>>    than under . Which actually is the other
>>>    option. So it's either:
>>>
>>>    
>>>  
>>>    
>>>  
>>>    
>>>
>>>    for net->hostIP, or it's:
>>>
>>>    
>>>  
>>>    
>>>
>>
> 
> In order to be consistent between all different hypervisors and
> interface connection types, any ip addresses and routes that are set to
> be visible on the host side of the interface "apparatus" must be under
> , and those that are set to be visible on the guest side must be
> directly under the top level of . Since the IP you're
> configuring in this patch will be seen in the guest, it needs to be at
> the toplevel (i.e. it will be parsed into net->guestIP).
> 
> 
>> I'm not convinced either.  If you don't like the fact that it's in
>> hostIP (even though we're setting up the backend of that device, not the
>> device itself, it's just the IP that the internal DHCP server should
>> send to the guest) and want to have it in guestIP (which would make
>> sense as well) then it should be:
>>
>>   
>>     
>>   
>>     
>>   
> 
> When I dropped into this code to add support for specifying the IP
> address and routes on the *host* side of a tap device/veth device (see
> commit 98fa8f3ef) for QEMU and LXC (along with the "peer IP" for the
> other end of a tap/veth, which is *not* the same thing!), I *wished*
> that we had added the new  and  elements for specifying
> guest-side  and  under  as you suggest, since
>  is intended to contain items configuring how the device appears
> on the guest side[*]. Unfortunately, the decision on the location of
> guest-side  was made *long* ago (at least prior to July of 2008, I
> didn't feel like spelunking any further), when  was added directly
> under  to support setting the guest-side IP address on Xen.
> 
> Many years later (2014), when cbossdonnat added support for setting the
> guest-side IP of  an LXC veth pair, he also used the same element in
> order to provide consistency. So, we are unable to put it under
> , and that decision was made sometime befor 2008,
> 
> 
> [*] Of course, the  element is used inconsistently within
>  when compared to other uses -  is used
> to specify the name of the tap/macvtap device *as it is known on the
> host side*.  has been described to me as a place to configure
> what things look like to the guest (e.g. the intended name for a disk
> device on the guest)(which is anyway ignored everywhere except Xen, but
> that's a different topic!), so using it to configure a name that's
> visible only to the host seems wrong, but it's been that way since
> sometime long before even I was involved with libvirt, so there is no
> changing it now :-P

Okay, I'll move it up so that it's under  directly.

> 
> 
>>
>> Which would cleanly correlate to that (and I also don't like the look of
>> it).  So I'll leave this to a further discussion.
> 
> I think it *would* have been good under , if anyone had foreseen
> the need for a  grouping (and consistently used it) from the
> beginning. But that train sailed into space long ago, so in order to be
> backward compatible and consistent, the guest-side IP/routes need to be
> directly under  (just like the guest-side PCI address for PCI
> devices is), and the host-side IP/routes need to go under  (just
> as host-side PCI addresses of  devices are).
> 
> 
>>
>>>    for net->guestIP. I went for the former one, but I
>>>    don't have a strong opinion on that.
> 
> That's good, because I *do*! :-P
> 
>>>
>>> docs/formatdomain.html.in  | 13 ++-
>>> docs/schemas/domaincommon.rng  |  5 +++
>>> src/conf/domain_conf.c |  5 +--
>>> .../qemuxml2argv-net-user-addr.xml | 42
>>> ++
>>> .../qemuxml2xmlout-net-user-addr.xml   |  1 +
>>> tests/qemuxml2xmltest.c    |  1 +
>>> 6 files changed, 64 insertions(+), 3 deletions(-)
>>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
>>> create mode 12
>>> tests/qemuxml2xmloutdata/qemuxml2xmlout-net-user-addr.xml
>>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index 8ca7637a4..65a8886ee 100644
>>> --- a/docs/formatdomain.ht

Re: [libvirt] [PATCH 4/5] conf: Allow usernet to have an address

2017-09-13 Thread Martin Kletzander

On Tue, Sep 12, 2017 at 01:35:08PM -0400, Laine Stump wrote:

On 09/12/2017 05:53 AM, Martin Kletzander wrote:

On Tue, Sep 12, 2017 at 11:32:52AM +0200, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1075520

Currently, all that users can specify for an interface type of
'user' is the common attributes: PCI address, NIC model (and
that's basically it). However, some need to configure other
address range than the default one.

Signed-off-by: Michal Privoznik 
---

Notes:
   Frankly, I'm not that convinced about this one. I mean,
   this puts IP addresses into net->hostIP while we are
   configuring guest side of the SLIRP. However, it just
   feels better to have the IP addresses under 
   than under . Which actually is the other
   option. So it's either:

   
 
   
 
   

   for net->hostIP, or it's:

   
 
   





In order to be consistent between all different hypervisors and
interface connection types, any ip addresses and routes that are set to
be visible on the host side of the interface "apparatus" must be under
, and those that are set to be visible on the guest side must be
directly under the top level of . Since the IP you're
configuring in this patch will be seen in the guest, it needs to be at
the toplevel (i.e. it will be parsed into net->guestIP).



I'm not convinced either.  If you don't like the fact that it's in
hostIP (even though we're setting up the backend of that device, not the
device itself, it's just the IP that the internal DHCP server should
send to the guest) and want to have it in guestIP (which would make
sense as well) then it should be:

  
    
  
    
  


When I dropped into this code to add support for specifying the IP
address and routes on the *host* side of a tap device/veth device (see
commit 98fa8f3ef) for QEMU and LXC (along with the "peer IP" for the
other end of a tap/veth, which is *not* the same thing!), I *wished*
that we had added the new  and  elements for specifying
guest-side  and  under  as you suggest, since
 is intended to contain items configuring how the device appears
on the guest side[*]. Unfortunately, the decision on the location of
guest-side  was made *long* ago (at least prior to July of 2008, I
didn't feel like spelunking any further), when  was added directly
under  to support setting the guest-side IP address on Xen.



I totally agree with ..., after I sent that mail Michal
explained to me that there already is such concept for LXC for example.
Had I known that, I wouldn't have talked about any .  But
thanks for thorough explanation anyway ;)


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/5] conf: Allow usernet to have an address

2017-09-12 Thread Laine Stump
On 09/12/2017 05:53 AM, Martin Kletzander wrote:
> On Tue, Sep 12, 2017 at 11:32:52AM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1075520
>>
>> Currently, all that users can specify for an interface type of
>> 'user' is the common attributes: PCI address, NIC model (and
>> that's basically it). However, some need to configure other
>> address range than the default one.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>
>> Notes:
>>    Frankly, I'm not that convinced about this one. I mean,
>>    this puts IP addresses into net->hostIP while we are
>>    configuring guest side of the SLIRP. However, it just
>>    feels better to have the IP addresses under 
>>    than under . Which actually is the other
>>    option. So it's either:
>>
>>    
>>  
>>    
>>  
>>    
>>
>>    for net->hostIP, or it's:
>>
>>    
>>  
>>    
>>
>

In order to be consistent between all different hypervisors and
interface connection types, any ip addresses and routes that are set to
be visible on the host side of the interface "apparatus" must be under
, and those that are set to be visible on the guest side must be
directly under the top level of . Since the IP you're
configuring in this patch will be seen in the guest, it needs to be at
the toplevel (i.e. it will be parsed into net->guestIP).


> I'm not convinced either.  If you don't like the fact that it's in
> hostIP (even though we're setting up the backend of that device, not the
> device itself, it's just the IP that the internal DHCP server should
> send to the guest) and want to have it in guestIP (which would make
> sense as well) then it should be:
>
>   
>     
>   
>     
>   

When I dropped into this code to add support for specifying the IP
address and routes on the *host* side of a tap device/veth device (see
commit 98fa8f3ef) for QEMU and LXC (along with the "peer IP" for the
other end of a tap/veth, which is *not* the same thing!), I *wished*
that we had added the new  and  elements for specifying
guest-side  and  under  as you suggest, since
 is intended to contain items configuring how the device appears
on the guest side[*]. Unfortunately, the decision on the location of
guest-side  was made *long* ago (at least prior to July of 2008, I
didn't feel like spelunking any further), when  was added directly
under  to support setting the guest-side IP address on Xen.

Many years later (2014), when cbossdonnat added support for setting the
guest-side IP of  an LXC veth pair, he also used the same element in
order to provide consistency. So, we are unable to put it under
, and that decision was made sometime befor 2008,


[*] Of course, the  element is used inconsistently within
 when compared to other uses -  is used
to specify the name of the tap/macvtap device *as it is known on the
host side*.  has been described to me as a place to configure
what things look like to the guest (e.g. the intended name for a disk
device on the guest)(which is anyway ignored everywhere except Xen, but
that's a different topic!), so using it to configure a name that's
visible only to the host seems wrong, but it's been that way since
sometime long before even I was involved with libvirt, so there is no
changing it now :-P


>
> Which would cleanly correlate to that (and I also don't like the look of
> it).  So I'll leave this to a further discussion.

I think it *would* have been good under , if anyone had foreseen
the need for a  grouping (and consistently used it) from the
beginning. But that train sailed into space long ago, so in order to be
backward compatible and consistent, the guest-side IP/routes need to be
directly under  (just like the guest-side PCI address for PCI
devices is), and the host-side IP/routes need to go under  (just
as host-side PCI addresses of  devices are).


>
>>    for net->guestIP. I went for the former one, but I
>>    don't have a strong opinion on that.

That's good, because I *do*! :-P

>>
>> docs/formatdomain.html.in  | 13 ++-
>> docs/schemas/domaincommon.rng  |  5 +++
>> src/conf/domain_conf.c |  5 +--
>> .../qemuxml2argv-net-user-addr.xml | 42
>> ++
>> .../qemuxml2xmlout-net-user-addr.xml   |  1 +
>> tests/qemuxml2xmltest.c    |  1 +
>> 6 files changed, 64 insertions(+), 3 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
>> create mode 12
>> tests/qemuxml2xmloutdata/qemuxml2xmlout-net-user-addr.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 8ca7637a4..65a8886ee 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -4701,7 +4701,14 @@
>>   starting from 10.0.2.15. The default router will be
>>   10.0.2.2 and the DNS server will be
>> 10.0.2.3.
>>   This networking is the only option for unprivileged users who
>> need their
>

Re: [libvirt] [PATCH 4/5] conf: Allow usernet to have an address

2017-09-12 Thread Martin Kletzander

On Tue, Sep 12, 2017 at 11:32:52AM +0200, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1075520

Currently, all that users can specify for an interface type of
'user' is the common attributes: PCI address, NIC model (and
that's basically it). However, some need to configure other
address range than the default one.

Signed-off-by: Michal Privoznik 
---

Notes:
   Frankly, I'm not that convinced about this one. I mean,
   this puts IP addresses into net->hostIP while we are
   configuring guest side of the SLIRP. However, it just
   feels better to have the IP addresses under 
   than under . Which actually is the other
   option. So it's either:

   
 
   
 
   

   for net->hostIP, or it's:

   
 
   



I'm not convinced either.  If you don't like the fact that it's in
hostIP (even though we're setting up the backend of that device, not the
device itself, it's just the IP that the internal DHCP server should
send to the guest) and want to have it in guestIP (which would make
sense as well) then it should be:

  

  

  

Which would cleanly correlate to that (and I also don't like the look of
it).  So I'll leave this to a further discussion.


   for net->guestIP. I went for the former one, but I
   don't have a strong opinion on that.

docs/formatdomain.html.in  | 13 ++-
docs/schemas/domaincommon.rng  |  5 +++
src/conf/domain_conf.c |  5 +--
.../qemuxml2argv-net-user-addr.xml | 42 ++
.../qemuxml2xmlout-net-user-addr.xml   |  1 +
tests/qemuxml2xmltest.c|  1 +
6 files changed, 64 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
create mode 12 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-user-addr.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8ca7637a4..65a8886ee 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4701,7 +4701,14 @@
  starting from 10.0.2.15. The default router will be
  10.0.2.2 and the DNS server will be 10.0.2.3.
  This networking is the only option for unprivileged users who need their
-  VMs to have outgoing access.
+  VMs to have outgoing access. However, since
+3.8.0, it is possible to override the default network by
+  including source element. The element can have 
ip
+  element for each IPv4 and IPv6. The element has family
+  attribute which accepts ipv4 and ipv6 values.
+  Then there's address attribute for specifying IP address
+  corresponding to the family. Optionally, the default prefix length can be
+  overridden via prefix attribute.



@@ -4711,6 +4718,10 @@
  ...
  

+
+  
+  
+
  

...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c9a4f7a9a..7b5292bd3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2428,6 +2428,11 @@
user
  
  
+
+  
+


This also allows  here, I would split the definition and
disallow that.  Just for the sake of pointless bugs flying by.


+  
+

  

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 676fc0f34..ef236757a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5214,12 +5214,13 @@ static int
virDomainNetDefValidate(const virDomainNetDef *net)
{
if ((net->hostIP.nroutes || net->hostIP.nips) &&
-net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) {
+net->type != VIR_DOMAIN_NET_TYPE_ETHERNET &&
+net->type != VIR_DOMAIN_NET_TYPE_USER) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("Invalid attempt to set network interface "
 "host-side IP route and/or address info on "
 "interface of type '%s'. This is only supported "
- "on interfaces of type 'ethernet'"),
+ "on interfaces of type 'ethernet' and 'user'"),


Same here, you even give the hint that routes are supported for
the type='user'


   virDomainNetTypeToString(net->type));
return -1;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
new file mode 100644
index 0..65362d8aa
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
@@ -0,0 +1,42 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-

[libvirt] [PATCH 4/5] conf: Allow usernet to have an address

2017-09-12 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1075520

Currently, all that users can specify for an interface type of
'user' is the common attributes: PCI address, NIC model (and
that's basically it). However, some need to configure other
address range than the default one.

Signed-off-by: Michal Privoznik 
---

Notes:
Frankly, I'm not that convinced about this one. I mean,
this puts IP addresses into net->hostIP while we are
configuring guest side of the SLIRP. However, it just
feels better to have the IP addresses under 
than under . Which actually is the other
option. So it's either:


  

  


for net->hostIP, or it's:


  


for net->guestIP. I went for the former one, but I
don't have a strong opinion on that.

 docs/formatdomain.html.in  | 13 ++-
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/domain_conf.c |  5 +--
 .../qemuxml2argv-net-user-addr.xml | 42 ++
 .../qemuxml2xmlout-net-user-addr.xml   |  1 +
 tests/qemuxml2xmltest.c|  1 +
 6 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
 create mode 12 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-user-addr.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8ca7637a4..65a8886ee 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4701,7 +4701,14 @@
   starting from 10.0.2.15. The default router will be
   10.0.2.2 and the DNS server will be 10.0.2.3.
   This networking is the only option for unprivileged users who need their
-  VMs to have outgoing access.
+  VMs to have outgoing access. However, since
+3.8.0, it is possible to override the default network by
+  including source element. The element can have 
ip
+  element for each IPv4 and IPv6. The element has family
+  attribute which accepts ipv4 and ipv6 values.
+  Then there's address attribute for specifying IP address
+  corresponding to the family. Optionally, the default prefix length can be
+  overridden via prefix attribute.
 
 
 
@@ -4711,6 +4718,10 @@
   ...
   
 
+
+  
+  
+
   
 
 ...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c9a4f7a9a..7b5292bd3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2428,6 +2428,11 @@
 user
   
   
+
+  
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 676fc0f34..ef236757a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5214,12 +5214,13 @@ static int
 virDomainNetDefValidate(const virDomainNetDef *net)
 {
 if ((net->hostIP.nroutes || net->hostIP.nips) &&
-net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) {
+net->type != VIR_DOMAIN_NET_TYPE_ETHERNET &&
+net->type != VIR_DOMAIN_NET_TYPE_USER) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Invalid attempt to set network interface "
  "host-side IP route and/or address info on "
  "interface of type '%s'. This is only supported "
- "on interfaces of type 'ethernet'"),
+ "on interfaces of type 'ethernet' and 'user'"),
virDomainNetTypeToString(net->type));
 return -1;
 }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
new file mode 100644
index 0..65362d8aa
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
@@ -0,0 +1,42 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+  
+  
+
+
+  
+
+
+  
+
+
+
+  
+  
+
+  
+  
+  
+
+
+
+
+  
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-user-addr.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-user-addr.xml
new file mode 12
index 0..fd909ec24
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-user-addr.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-net-user-addr.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0a87cedf2..99b15ad0c 100644
--- a/tests/qemuxml2xmltest.c
++