9:41 p.m.
Howdy!
I spent some time scratching my head this evening, as libvirt was
stripping the target/@dev entry from my interface[@type='ethernet'].
Turns out that this was caused by the interface name containing a
period, which is rejected by isValidIfname(). As the kernel allows the
creation of such interfaces, it is improper for libvirt to reject them.
The attached one-liner (built against RHEL6b1's libvirt but still
applicable against current master as of 10c681622) fixes this issue.
10:05 p.m.
On 05/19/2010 10:41 PM, Charles Duffy wrote:
Howdy!
I spent some time scratching my head this evening, as libvirt was
stripping the target/@dev entry from my interface[@type='ethernet'].
Turns out that this was caused by the interface name containing a
period, which is rejected by isValidIfname(). As the kernel allows the
creation of such interfaces, it is improper for libvirt to reject them.
The attached one-liner (built against RHEL6b1's libvirt but still
applicable against current master as of 10c681622) fixes this issue.
I think we need to add ":" there as well (at least - maybe there are
others), don't we?
11:54 p.m.
New subject: [libvirt] [PATCH v2] dots should be valid characters in interface names
On 05/19/2010 10:05 PM, Laine Stump wrote:
On 05/19/2010 10:41 PM, Charles Duffy wrote:
> The attached one-liner (built against RHEL6b1's libvirt but still
> applicable against current master as of 10c681622) fixes this issue.
I think we need to add ":" there as well (at least - maybe there are
others), don't we?
In dev_valid_name() in net/core/dev.c, the following restrictions are
enforced as of 2.6.27:
- Empty strings are disallowed
- Length must not meet or exceed 16 (IFNAMSIZ)
- '.' and '..' are each invalid as freestanding names (but accepted as
part of a longer string)
- whitespace and '/' characters are prohibited at any point in the string
...and those are the only limitations in play.
Frankly, I'm a bit surprised colons are allowed -- my intuition was that
they would be permitted only for named IPs rather than devices -- but
such is the logic given here, and a bit of experimentation shows that
the kernel accepts them (though they trigger bugs in iproute2, which
appears in at least one place to make the same assumption I did; fun!)
A revised patch is attached. This lifts its logic from its kernel
counterpart, and is updated only to permit forward slashes (which, while
disallowed for interface names with the kernel, are required for
*device* names -- for which the ESX driver happens to overload this
field. Ugh).
12:06 a.m.
New subject: [libvirt] [PATCH v3] dots should be valid characters in interface names
On 05/19/2010 11:54 PM, Charles Duffy wrote:
A revised patch is attached. This lifts its logic from its kernel
counterpart, and is updated only to permit forward slashes (which, while
disallowed for interface names with the kernel, are required for
*device* names -- for which the ESX driver happens to overload this
field. Ugh).
Failed to run "make syntax-check" on the last rev. Oops.
9:12 a.m.
New subject: [libvirt] [PATCH v3] dots should be valid characters in interface names
On 05/20/2010 01:06 AM, Charles Duffy wrote:
On 05/19/2010 11:54 PM, Charles Duffy wrote:
> A revised patch is attached. This lifts its logic from its kernel
> counterpart, and is updated only to permit forward slashes (which, while
> disallowed for interface names with the kernel, are required for
> *device* names -- for which the ESX driver happens to overload this
> field. Ugh).
Hm. You know, I'm wondering if we should have "isValidIfname" at all in
domain_conf.c. The thing is that if you think about different platforms
that this could be running on (Windows, Linux, Solaris, etc), and the
number of different drivers that go through here (Qemu, Xen, ESX, Virtualbox,
Phyp, openvz, ONE, UML), it seems like we have to allow everything to
cover all bases. Indeed, there is very little else in domain_conf.c that
does validity checking, probably for just this reason.
Charles, Laine, what do you think about just removing this check completely?
--
Chris Lalancette
9:23 a.m.
New subject: [libvirt] [PATCH v3] dots should be valid characters in interface names
On Thu, May 20, 2010 at 10:12:35AM -0400, Chris Lalancette wrote:
On 05/20/2010 01:06 AM, Charles Duffy wrote:
> On 05/19/2010 11:54 PM, Charles Duffy wrote:
>> A revised patch is attached. This lifts its logic from its kernel
>> counterpart, and is updated only to permit forward slashes (which, while
>> disallowed for interface names with the kernel, are required for
>> *device* names -- for which the ESX driver happens to overload this
>> field. Ugh).
Hm. You know, I'm wondering if we should have "isValidIfname" at all in
domain_conf.c. The thing is that if you think about different platforms
that this could be running on (Windows, Linux, Solaris, etc), and the
number of different drivers that go through here (Qemu, Xen, ESX, Virtualbox,
Phyp, openvz, ONE, UML), it seems like we have to allow everything to
cover all bases. Indeed, there is very little else in domain_conf.c that
does validity checking, probably for just this reason.
Yep, if the check is platform/hypervisor specific, then it shouldn't be
in the XML parsers. The parsers should only attempt to validate syntactic
correctness, not semantic correctness (with exception of semantics that
can be validated based on the capabilities data which accurately reflects
the hypervisor specific rules in that case)
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
12:11 p.m.
New subject: [libvirt] [PATCH v3] dots should be valid characters in interface names
On 05/20/2010 10:12 AM, Chris Lalancette wrote:
On 05/20/2010 01:06 AM, Charles Duffy wrote:
> On 05/19/2010 11:54 PM, Charles Duffy wrote:
>
>> A revised patch is attached. This lifts its logic from its kernel
>> counterpart, and is updated only to permit forward slashes (which, while
>> disallowed for interface names with the kernel, are required for
>> *device* names -- for which the ESX driver happens to overload this
>> field. Ugh).
>>
Hm. You know, I'm wondering if we should have "isValidIfname" at all in
domain_conf.c. The thing is that if you think about different platforms
that this could be running on (Windows, Linux, Solaris, etc), and the
number of different drivers that go through here (Qemu, Xen, ESX, Virtualbox,
Phyp, openvz, ONE, UML), it seems like we have to allow everything to
cover all bases. Indeed, there is very little else in domain_conf.c that
does validity checking, probably for just this reason.
Charles, Laine, what do you think about just removing this check completely?
interface_conf.c also parses interface names, and has no validity
checking - anything inside the quotes is acceptable. So yeah, I have no
problem with not having a check.
(All this talk about valid characters has me thinking about the plethora
of websites that have validity checkers for email addresses that don't
allow "+". Grrr.)
4:06 p.m.
New subject: [libvirt] [PATCH v4] dots should be valid characters in interface names
On 05/20/2010 09:12 AM, Chris Lalancette wrote:
On 05/20/2010 01:06 AM, Charles Duffy wrote:
> On 05/19/2010 11:54 PM, Charles Duffy wrote:
>> A revised patch is attached. This lifts its logic from its kernel
>> counterpart, and is updated only to permit forward slashes (which, while
>> disallowed for interface names with the kernel, are required for
>> *device* names -- for which the ESX driver happens to overload this
>> field. Ugh).
Hm. You know, I'm wondering if we should have "isValidIfname" at all in
domain_conf.c. The thing is that if you think about different platforms
that this could be running on (Windows, Linux, Solaris, etc), and the
number of different drivers that go through here (Qemu, Xen, ESX, Virtualbox,
Phyp, openvz, ONE, UML), it seems like we have to allow everything to
cover all bases. Indeed, there is very little else in domain_conf.c that
does validity checking, probably for just this reason.
Charles, Laine, what do you think about just removing this check completely?
WORKSFORME as well.
I'm presuming that it's still appropriate to check against an empty
string. Does the attached look right? If so, I'll update the associated
RHEL6 ticket [https://bugzilla.redhat.com/show_bug.cgi?id=593907]
appropriately.
4:45 p.m.
New subject: [libvirt] [PATCH v4] dots should be valid characters in interface names
On 05/20/2010 03:06 PM, Charles Duffy wrote:
> Charles, Laine, what do you think about just removing this check
> completely?
WORKSFORME as well.
I'm presuming that it's still appropriate to check against an empty
string. Does the attached look right? If so, I'll update the associated
RHEL6 ticket [https://bugzilla.redhat.com/show_bug.cgi?id=593907]
appropriately.
Looks like Chris posted a similar patch, but I like yours better. But
why the additional inclusion of "c-ctype.h"?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5:11 p.m.
New subject: [libvirt] [PATCH v5] dots should be valid characters in interface names
On 05/20/2010 04:45 PM, Eric Blake wrote:
On 05/20/2010 03:06 PM, Charles Duffy wrote:
>> Charles, Laine, what do you think about just removing this check
>> completely?
>
> WORKSFORME as well.
>
> I'm presuming that it's still appropriate to check against an empty
> string. Does the attached look right? If so, I'll update the associated
> RHEL6 ticket [https://bugzilla.redhat.com/show_bug.cgi?id=593907]
> appropriately.
Looks like Chris posted a similar patch, but I like yours better. But
why the additional inclusion of "c-ctype.h"?
Accidental -- updated patch attached -- though I'm perfectly happy with
Chris's version too (though it probably ought to clean up the now-unused
VALID_IFNAME_CHARS).
5277
days inactive
5277
days old
9 comments
5 participants
participants (5)
-
Charles Duffy -
Chris Lalancette -
Daniel P. Berrange -
Eric Blake -
Laine Stump