11:34 p.m.
This would have been just one small patch, but there was a deficiency
in the low level functions to save and restore pre-existing state of a
virtual function.
I have tested these patches on a RHEL6 machine with an SR-IOV adapter,
and it properly handled the save/set .. [attach-to-guest] .. [detach]
.. restore cycle both when the original state of the VF had a vlan tag
and not, and when the guest was requesting a vlan tag, and not.
(Now I'm wondering if this would also work for macvtap "passthrough"
mode when the adapter used is a VF - it looks like the only thing
stopping it is that the code to save / set / restore in that mode uses
the device name of the vf, rather than referring to it as <physical
device> vf <n>; since only VFs understand setting the vlan tag, the
lower level code just assumes it can't be done)
11:34 p.m.
New subject: [libvirt] [PATCH 1/2] util: properly save/restore original vlan tag for VFs
When a network device that is a VF of an SR-IOV card was assigned to a
guest using <interface type='hostdev'>, only the MAC address was being
saved/restored, but the VLAN tag was left untouched. Up to now we
haven't actually used vlan tags on SR-IOV devices, so the guest would
have used whatever was set, and left it the same at the end.
The patch following this one will hook up the <vlan> element from the
interface config, so save/restore of the device state needs to also
include the vlan tag.
MAC address is being saved as a simple ASCII string in a file named
for the device under /var/run. The VLAN tag is now just added at the
end of that file, after a newline. It might be nicer if the file was
XML (in case it ever gets more complicated) but at the moment there's
nothing else on the horizon, and this makes backward compatibility
easier.
---
src/util/virnetdev.c | 56 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index f1ee0a4..25bdf01 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1533,33 +1533,41 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf,
int vlanid,
const char *stateDir)
{
+ int ret = -1;
virMacAddr oldmac;
int oldvlanid = -1;
char *path = NULL;
char macstr[VIR_MAC_STRING_BUFLEN];
+ char *fileData = NULL;
int ifindex = -1;
if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0)
- return -1;
+ goto cleanup;
if (virAsprintf(&path, "%s/%s_vf%d",
stateDir, pflinkdev, vf) < 0) {
virReportOOMError();
- return -1;
+ goto cleanup;
}
- virMacAddrFormat(&oldmac, macstr);
- if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY) < 0) {
- virReportSystemError(errno, _("Unable to preserve mac for pf = %s,"
- " vf = %d"), pflinkdev, vf);
- VIR_FREE(path);
- return -1;
+ if (virAsprintf(&fileData, "%s\n%d\n",
+ virMacAddrFormat(&oldmac, macstr), oldvlanid) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (virFileWriteStr(path, fileData, O_CREAT|O_TRUNC|O_WRONLY) < 0) {
+ virReportSystemError(errno, _("Unable to preserve mac/vlan tag "
+ "for pf = %s, vf = %d"), pflinkdev, vf);
+ goto cleanup;
}
- VIR_FREE(path);
-
- return virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
+ ret = virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
macaddress, vlanid, NULL);
+
+cleanup:
+ VIR_FREE(path);
+ VIR_FREE(fileData);
+ return ret;
}
static int
@@ -1567,8 +1575,9 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
const char *stateDir)
{
int rc = -1;
- char *macstr = NULL;
char *path = NULL;
+ char *fileData = NULL;
+ char *vlan = NULL;
virMacAddr oldmac;
int vlanid = -1;
int ifindex = -1;
@@ -1579,14 +1588,29 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
return rc;
}
- if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN, &macstr) < 0) {
+ if (virFileReadAll(path, 128, &fileData) < 0) {
goto cleanup;
}
- if (virMacAddrParse(macstr, &oldmac) != 0) {
+ if ((vlan = strchr(fileData, '\n'))) {
+ char *endptr;
+
+ *vlan++ = 0; /* NULL terminate the mac address */
+ if (*vlan) {
+ if ((virStrToLong_i(vlan, &endptr, 10, &vlanid) < 0) ||
+ (endptr && *endptr != '\n' && *endptr != 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse vlan tag from '%s'"),
+ vlan);
+ goto cleanup;
+ }
+ }
+ }
+
+ if (virMacAddrParse(fileData, &oldmac) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse MAC address from '%s'"),
- macstr);
+ fileData);
goto cleanup;
}
@@ -1597,7 +1621,7 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
cleanup:
VIR_FREE(path);
- VIR_FREE(macstr);
+ VIR_FREE(fileData);
return rc;
}
--
1.7.11.4
3:34 a.m.
New subject: [libvirt] [PATCH 1/2] util: properly save/restore original vlan tag for VFs
On 2012年08月16日 12:34, Laine Stump wrote:
When a network device that is a VF of an SR-IOV card was assigned to
a
guest using<interface type='hostdev'>, only the MAC address was being
saved/restored, but the VLAN tag was left untouched. Up to now we
haven't actually used vlan tags on SR-IOV devices, so the guest would
have used whatever was set, and left it the same at the end.
The patch following this one will hook up the<vlan> element from the
interface config, so save/restore of the device state needs to also
include the vlan tag.
MAC address is being saved as a simple ASCII string in a file named
for the device under /var/run. The VLAN tag is now just added at the
end of that file, after a newline. It might be nicer if the file was
XML (in case it ever gets more complicated) but at the moment there's
nothing else on the horizon, and this makes backward compatibility
easier.
---
src/util/virnetdev.c | 56 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index f1ee0a4..25bdf01 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1533,33 +1533,41 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf,
int vlanid,
const char *stateDir)
{
+ int ret = -1;
virMacAddr oldmac;
int oldvlanid = -1;
char *path = NULL;
char macstr[VIR_MAC_STRING_BUFLEN];
+ char *fileData = NULL;
int ifindex = -1;
if (virNetDevGetVfConfig(pflinkdev, vf,&oldmac,&oldvlanid)< 0)
- return -1;
+ goto cleanup;
if (virAsprintf(&path, "%s/%s_vf%d",
stateDir, pflinkdev, vf)< 0) {
virReportOOMError();
- return -1;
+ goto cleanup;
}
- virMacAddrFormat(&oldmac, macstr);
- if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY)< 0) {
- virReportSystemError(errno, _("Unable to preserve mac for pf = %s,"
- " vf = %d"), pflinkdev, vf);
- VIR_FREE(path);
- return -1;
+ if (virAsprintf(&fileData, "%s\n%d\n",
+ virMacAddrFormat(&oldmac, macstr), oldvlanid)< 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (virFileWriteStr(path, fileData, O_CREAT|O_TRUNC|O_WRONLY)< 0) {
+ virReportSystemError(errno, _("Unable to preserve mac/vlan tag "
+ "for pf = %s, vf = %d"), pflinkdev,
vf);
+ goto cleanup;
}
- VIR_FREE(path);
-
- return virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
+ ret = virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
macaddress, vlanid, NULL);
+
+cleanup:
+ VIR_FREE(path);
+ VIR_FREE(fileData);
+ return ret;
}
static int
@@ -1567,8 +1575,9 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
const char *stateDir)
{
int rc = -1;
- char *macstr = NULL;
char *path = NULL;
+ char *fileData = NULL;
+ char *vlan = NULL;
virMacAddr oldmac;
int vlanid = -1;
int ifindex = -1;
@@ -1579,14 +1588,29 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
return rc;
}
- if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN,&macstr)< 0) {
+ if (virFileReadAll(path, 128,&fileData)< 0) {
goto cleanup;
}
- if (virMacAddrParse(macstr,&oldmac) != 0) {
+ if ((vlan = strchr(fileData, '\n'))) {
+ char *endptr;
+
+ *vlan++ = 0; /* NULL terminate the mac address */
s/terminate/terminates/
+ if (*vlan) {
+ if ((virStrToLong_i(vlan,&endptr, 10,&vlanid)< 0) ||
+ (endptr&& *endptr != '\n'&& *endptr != 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse vlan tag from '%s'"),
+ vlan);
+ goto cleanup;
+ }
+ }
+ }
+
+ if (virMacAddrParse(fileData,&oldmac) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse MAC address from '%s'"),
- macstr);
+ fileData);
goto cleanup;
}
@@ -1597,7 +1621,7 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
cleanup:
VIR_FREE(path);
- VIR_FREE(macstr);
+ VIR_FREE(fileData);
return rc;
}
Looks good for me, ACK.
9:28 a.m.
New subject: [libvirt] [PATCH 1/2] util: properly save/restore original vlan tag for VFs
On 08/16/2012 02:34 AM, Osier Yang wrote:
>
> - if (virMacAddrParse(macstr,&oldmac) != 0) {
> + if ((vlan = strchr(fileData, '\n'))) {
> + char *endptr;
> +
> + *vlan++ = 0; /* NULL terminate the mac address */
s/terminate/terminates/
Actually, 'terminate' is correct - this is a comment describing what the
assignment is good for (convert arbitrary contents into 0 in order to
NUL-terminate the vlan variable), not the vlan format itself (a valid
vlan exists when NUL terminates the string).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:34 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: support setting vlan tag for <interface type='hostdev'>
The underlying function to set the vlan tag of an SR-IOV network
device was already in place (although an extra patch to save/restore
the original vlan tag was needed), and recent patches added the
ability to configure a vlan tag. This patch just ties those two
together.
An SR-IOV device doesn't support vlan trunking, so if anyone tries to
configure more than a single tag, or set the trunk flag, and error is
logged.
---
src/qemu/qemu_hostdev.c | 40 ++++++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 7619fd0..46c84b5 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -301,6 +301,7 @@ qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
char *stateDir)
{
char *linkdev = NULL;
+ virNetDevVlanPtr vlan;
virNetDevVPortProfilePtr virtPort;
int ret = -1;
int vf = -1;
@@ -319,19 +320,46 @@ qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
if (qemuDomainHostdevNetDevice(hostdev, &linkdev, &vf) < 0)
return ret;
+ vlan = virDomainNetGetActualVlan(hostdev->parent.data.net);
virtPort = virDomainNetGetActualVirtPortProfile(
hostdev->parent.data.net);
- if (virtPort)
+ if (virtPort) {
+ if (vlan) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("direct setting of the vlan tag is not allowed "
+ "for hostdev devices using %s mode"),
+ virNetDevVPortTypeToString(virtPort->virtPortType));
+ goto cleanup;
+ }
ret = qemuDomainHostdevNetConfigVirtPortProfile(linkdev, vf,
virtPort, &hostdev->parent.data.net->mac, uuid,
port_profile_associate);
- else
- /* Set only mac */
+ } else {
+ /* Set only mac and vlan */
+ if (vlan) {
+ if (vlan->nTags != 1 || vlan->trunk) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("vlan trunking is not supported "
+ "by SR-IOV network devices"));
+ goto cleanup;
+ }
+ if (vf == -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("vlan can only be set for SR-IOV VFs, but "
+ "%s is not a VF"), linkdev);
+ goto cleanup;
+ }
+ vlanid = vlan->tag[0];
+ } else if (vf >= 0) {
+ vlanid = 0; /* assure any current vlan tag is reset */
+ }
+
ret = virNetDevReplaceNetConfig(linkdev, vf,
- &hostdev->parent.data.net->mac,
vlanid,
- stateDir);
+ &hostdev->parent.data.net->mac,
+ vlanid, stateDir);
+ }
+cleanup:
VIR_FREE(linkdev);
-
return ret;
}
--
1.7.11.4
3:39 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: support setting vlan tag for <interface type='hostdev'>
On 2012年08月16日 12:34, Laine Stump wrote:
The underlying function to set the vlan tag of an SR-IOV network
device was already in place (although an extra patch to save/restore
the original vlan tag was needed), and recent patches added the
ability to configure a vlan tag. This patch just ties those two
together.
An SR-IOV device doesn't support vlan trunking, so if anyone tries to
configure more than a single tag, or set the trunk flag, and error is
logged.
---
src/qemu/qemu_hostdev.c | 40 ++++++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 7619fd0..46c84b5 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -301,6 +301,7 @@ qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
char *stateDir)
{
char *linkdev = NULL;
+ virNetDevVlanPtr vlan;
virNetDevVPortProfilePtr virtPort;
int ret = -1;
int vf = -1;
@@ -319,19 +320,46 @@ qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
if (qemuDomainHostdevNetDevice(hostdev,&linkdev,&vf)< 0)
return ret;
+ vlan = virDomainNetGetActualVlan(hostdev->parent.data.net);
virtPort = virDomainNetGetActualVirtPortProfile(
hostdev->parent.data.net);
- if (virtPort)
+ if (virtPort) {
+ if (vlan) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("direct setting of the vlan tag is not allowed "
+ "for hostdev devices using %s mode"),
+ virNetDevVPortTypeToString(virtPort->virtPortType));
+ goto cleanup;
+ }
ret = qemuDomainHostdevNetConfigVirtPortProfile(linkdev, vf,
virtPort,&hostdev->parent.data.net->mac, uuid,
port_profile_associate);
- else
- /* Set only mac */
+ } else {
+ /* Set only mac and vlan */
+ if (vlan) {
+ if (vlan->nTags != 1 || vlan->trunk) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("vlan trunking is not supported "
+ "by SR-IOV network devices"));
+ goto cleanup;
+ }
+ if (vf == -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("vlan can only be set for SR-IOV VFs, but "
+ "%s is not a VF"), linkdev);
+ goto cleanup;
+ }
+ vlanid = vlan->tag[0];
+ } else if (vf>= 0) {
+ vlanid = 0; /* assure any current vlan tag is reset */
+ }
+
ret = virNetDevReplaceNetConfig(linkdev, vf,
-&hostdev->parent.data.net->mac, vlanid,
- stateDir);
+&hostdev->parent.data.net->mac,
+ vlanid, stateDir);
+ }
+cleanup:
VIR_FREE(linkdev);
-
return ret;
}
ACK.
4481
days inactive
4481
days old
5 comments
3 participants
participants (3)
-
Eric Blake -
Laine Stump -
Osier Yang