[libvirt] [PATCH 0/6] Add multiqueue support for macvtaps
Some patches are easier to review when showed with --ignore-space-change. Michal Privoznik (6): virNetDevMacVLanCreateWithVPortProfile: Turn vnet_hdr into flag virNetDevMacVLanTapOpen: Slightly rework virNetDevMacVLanTapOpen: Rework to support multiple FDs virNetDevMacVLanTapSetup: Rework to support multiple FDs virNetDevMacVLanCreateWithVPortProfile: Rework to support multiple FDs qemu: Enable multiqueue for macvtaps src/lxc/lxc_process.c | 3 +- src/qemu/qemu_command.c | 61 +++++++++------- src/qemu/qemu_command.h | 2 + src/qemu/qemu_hotplug.c | 16 +++-- src/util/virnetdevmacvlan.c | 171 ++++++++++++++++++++++++-------------------- src/util/virnetdevmacvlan.h | 13 ++-- 6 files changed, 150 insertions(+), 116 deletions(-) -- 2.4.10
So yet again one of integer arguments that we use as a boolean. Since the argument count of the function is unbearably long enough, lets turn those booleans into flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_command.c | 5 ++--- src/util/virnetdevmacvlan.c | 4 +--- src/util/virnetdevmacvlan.h | 9 +++++---- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 57e3880..0ada6e4 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -344,7 +344,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, net->ifname, &net->mac, linkdev, virDomainNetGetActualDirectMode(net), - false, def->uuid, + def->uuid, prof, &res_ifname, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4ff31dc..55809e9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -224,18 +224,17 @@ qemuPhysIfaceConnect(virDomainDefPtr def, { int rc; char *res_ifname = NULL; - int vnet_hdr = 0; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP; if (net->model && STREQ(net->model, "virtio")) - vnet_hdr = 1; + macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR; rc = virNetDevMacVLanCreateWithVPortProfile( net->ifname, &net->mac, virDomainNetGetActualDirectDev(net), virDomainNetGetActualDirectMode(net), - vnet_hdr, def->uuid, + def->uuid, virDomainNetGetActualVirtPortProfile(net), &res_ifname, vmop, cfg->stateDir, diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index de345e6..9384b9f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -727,7 +727,6 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, * @macaddress: The MAC address for the macvtap device * @linkdev: The interface name of the NIC to connect to the external bridge * @mode: int describing the mode for 'bridge', 'vepa', 'private' or 'passthru'. - * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it * @vmuuid: The UUID of the VM the macvtap belongs to * @virtPortProfile: pointer to object holding the virtual port profile data * @res_ifname: Pointer to a string pointer where the actual name of the @@ -743,7 +742,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const virMacAddr *macaddress, const char *linkdev, virNetDevMacVLanMode mode, - int vnet_hdr, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, char **res_ifname, @@ -764,6 +762,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const char *cr_ifname = NULL; int ret; int vf = -1; + bool vnet_hdr = flags & VIR_NETDEV_MACVLAN_VNET_HDR; macvtapMode = modeMap[mode]; @@ -1017,7 +1016,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname ATTRIBUTE_UNUSED, const virMacAddr *macaddress ATTRIBUTE_UNUSED, const char *linkdev ATTRIBUTE_UNUSED, virNetDevMacVLanMode mode ATTRIBUTE_UNUSED, - int vnet_hdr ATTRIBUTE_UNUSED, const unsigned char *vmuuid ATTRIBUTE_UNUSED, virNetDevVPortProfilePtr virtPortProfile ATTRIBUTE_UNUSED, char **res_ifname ATTRIBUTE_UNUSED, diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index 298e522..b04fc8c 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -41,11 +41,13 @@ typedef enum { VIR_ENUM_DECL(virNetDevMacVLanMode) typedef enum { - VIR_NETDEV_MACVLAN_CREATE_NONE = 0, + VIR_NETDEV_MACVLAN_CREATE_NONE = 0, /* Create with a tap device */ - VIR_NETDEV_MACVLAN_CREATE_WITH_TAP = 1 << 0, + VIR_NETDEV_MACVLAN_CREATE_WITH_TAP = 1 << 0, /* Bring the interface up */ - VIR_NETDEV_MACVLAN_CREATE_IFUP = 1 << 1, + VIR_NETDEV_MACVLAN_CREATE_IFUP = 1 << 1, + /* Enable VNET_HDR */ + VIR_NETDEV_MACVLAN_VNET_HDR = 1 << 2, } virNetDevMacVLanCreateFlags; int virNetDevMacVLanCreate(const char *ifname, @@ -64,7 +66,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname, const virMacAddr *macaddress, const char *linkdev, virNetDevMacVLanMode mode, - int vnet_hdr, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, char **res_ifname, -- 2.4.10
On 12/04/2015 07:30 AM, Michal Privoznik wrote:
So yet again one of integer arguments that we use as a boolean. Since the argument count of the function is unbearably long enough, lets turn those booleans into flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_command.c | 5 ++--- src/util/virnetdevmacvlan.c | 4 +--- src/util/virnetdevmacvlan.h | 9 +++++---- 4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 57e3880..0ada6e4 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -344,7 +344,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, net->ifname, &net->mac, linkdev, virDomainNetGetActualDirectMode(net), - false, def->uuid, + def->uuid, prof, &res_ifname, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4ff31dc..55809e9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -224,18 +224,17 @@ qemuPhysIfaceConnect(virDomainDefPtr def, { int rc; char *res_ifname = NULL; - int vnet_hdr = 0; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
if (net->model && STREQ(net->model, "virtio")) - vnet_hdr = 1; + macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR;
rc = virNetDevMacVLanCreateWithVPortProfile( net->ifname, &net->mac, virDomainNetGetActualDirectDev(net), virDomainNetGetActualDirectMode(net), - vnet_hdr, def->uuid, + def->uuid, virDomainNetGetActualVirtPortProfile(net), &res_ifname, vmop, cfg->stateDir, diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index de345e6..9384b9f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -727,7 +727,6 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, * @macaddress: The MAC address for the macvtap device * @linkdev: The interface name of the NIC to connect to the external bridge * @mode: int describing the mode for 'bridge', 'vepa', 'private' or 'passthru'. - * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it * @vmuuid: The UUID of the VM the macvtap belongs to * @virtPortProfile: pointer to object holding the virtual port profile data * @res_ifname: Pointer to a string pointer where the actual name of the @@ -743,7 +742,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const virMacAddr *macaddress, const char *linkdev, virNetDevMacVLanMode mode, - int vnet_hdr, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, char **res_ifname, @@ -764,6 +762,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const char *cr_ifname = NULL; int ret; int vf = -1; + bool vnet_hdr = flags & VIR_NETDEV_MACVLAN_VNET_HDR;
macvtapMode = modeMap[mode];
@@ -1017,7 +1016,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname ATTRIBUTE_UNUSED, const virMacAddr *macaddress ATTRIBUTE_UNUSED, const char *linkdev ATTRIBUTE_UNUSED, virNetDevMacVLanMode mode ATTRIBUTE_UNUSED, - int vnet_hdr ATTRIBUTE_UNUSED, const unsigned char *vmuuid ATTRIBUTE_UNUSED, virNetDevVPortProfilePtr virtPortProfile ATTRIBUTE_UNUSED, char **res_ifname ATTRIBUTE_UNUSED, diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index 298e522..b04fc8c 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -41,11 +41,13 @@ typedef enum { VIR_ENUM_DECL(virNetDevMacVLanMode)
typedef enum { - VIR_NETDEV_MACVLAN_CREATE_NONE = 0, + VIR_NETDEV_MACVLAN_CREATE_NONE = 0, /* Create with a tap device */ - VIR_NETDEV_MACVLAN_CREATE_WITH_TAP = 1 << 0, + VIR_NETDEV_MACVLAN_CREATE_WITH_TAP = 1 << 0, /* Bring the interface up */ - VIR_NETDEV_MACVLAN_CREATE_IFUP = 1 << 1, + VIR_NETDEV_MACVLAN_CREATE_IFUP = 1 << 1, + /* Enable VNET_HDR */ + VIR_NETDEV_MACVLAN_VNET_HDR = 1 << 2, } virNetDevMacVLanCreateFlags;
Any reason for the alignment adjustment? If not, it's just fodder for creating merge conflicts later. Aside from that, ACK.
On 04.12.2015 16:41, Laine Stump wrote:
On 12/04/2015 07:30 AM, Michal Privoznik wrote:
So yet again one of integer arguments that we use as a boolean. Since the argument count of the function is unbearably long enough, lets turn those booleans into flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_command.c | 5 ++--- src/util/virnetdevmacvlan.c | 4 +--- src/util/virnetdevmacvlan.h | 9 +++++---- 4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index 298e522..b04fc8c 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -41,11 +41,13 @@ typedef enum { VIR_ENUM_DECL(virNetDevMacVLanMode) typedef enum { - VIR_NETDEV_MACVLAN_CREATE_NONE = 0, + VIR_NETDEV_MACVLAN_CREATE_NONE = 0, /* Create with a tap device */ - VIR_NETDEV_MACVLAN_CREATE_WITH_TAP = 1 << 0, + VIR_NETDEV_MACVLAN_CREATE_WITH_TAP = 1 << 0, /* Bring the interface up */ - VIR_NETDEV_MACVLAN_CREATE_IFUP = 1 << 1, + VIR_NETDEV_MACVLAN_CREATE_IFUP = 1 << 1, + /* Enable VNET_HDR */ + VIR_NETDEV_MACVLAN_VNET_HDR = 1 << 2, } virNetDevMacVLanCreateFlags;
Any reason for the alignment adjustment? If not, it's just fodder for creating merge conflicts later.
No, it's just because my TABs are that wide. Nevermind.
Aside from that, ACK.
Michal
There are few outdated things. Firstly, we don't need to undergo the torture of fopen, fscanf and fclose when we have nice wrapper over that: virFileReadAll. Secondly, we can use dynamically allocated buffer for the interface index. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 9384b9f..c1a5f0f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -237,40 +237,28 @@ int virNetDevMacVLanTapOpen(const char *ifname, int retries) { int ret = -1; - FILE *file = NULL; char *path; int ifindex; - char tapname[50]; + char *tapname = NULL; + char *buf = NULL; + char *c; int tapfd; if (virNetDevSysfsFile(&path, ifname, "ifindex") < 0) return -1; - file = fopen(path, "r"); - - if (!file) { - virReportSystemError(errno, - _("cannot open macvtap file %s to determine " - "interface index"), path); + if (virFileReadAll(path, sizeof(buf), &buf) < 0) goto cleanup; - } - if (fscanf(file, "%d", &ifindex) != 1) { + if (virStrToLong_i(buf, &c, 10, &ifindex) < 0 || *c != '\n') { virReportSystemError(errno, "%s", _("cannot determine macvtap's tap device " - "interface index")); + "interface index")); goto cleanup; } - VIR_FORCE_FCLOSE(file); - - if (snprintf(tapname, sizeof(tapname), - "/dev/tap%d", ifindex) >= sizeof(tapname)) { - virReportSystemError(errno, - "%s", - _("internal buffer for tap device is too small")); + if (virAsprintf(&tapname, "/dev/tap%d", ifindex) < 0) goto cleanup; - } while (1) { /* may need to wait for udev to be done */ @@ -292,7 +280,8 @@ int virNetDevMacVLanTapOpen(const char *ifname, ret = tapfd; cleanup: VIR_FREE(path); - VIR_FORCE_FCLOSE(file); + VIR_FREE(tapname); + VIR_FREE(buf); return ret; } -- 2.4.10
On 12/04/2015 07:30 AM, Michal Privoznik wrote:
There are few outdated things. Firstly, we don't need to undergo the torture of fopen, fscanf and fclose when we have nice wrapper over that: virFileReadAll. Secondly, we can use dynamically allocated buffer for the interface index.
Nothing against your changes to the existing function (ACK to that), but why is it reading sysfs for the ifindex? Why not just use virNetDevGetIndex(), as we do everywhere else in libvirt? (For that matter, I'm betting that the response message to the netlink request that creates the macvtap device will already contain the ifindex of the newly created device, but it would take more re-working of the code to carry that up from virNetDevMacVLanCreate() and over into virNetDevMacVLanTapOpen(), so likely not worth the small efficiency gain).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 9384b9f..c1a5f0f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -237,40 +237,28 @@ int virNetDevMacVLanTapOpen(const char *ifname, int retries) { int ret = -1; - FILE *file = NULL; char *path; int ifindex; - char tapname[50]; + char *tapname = NULL; + char *buf = NULL; + char *c; int tapfd;
if (virNetDevSysfsFile(&path, ifname, "ifindex") < 0) return -1;
- file = fopen(path, "r"); - - if (!file) { - virReportSystemError(errno, - _("cannot open macvtap file %s to determine " - "interface index"), path); + if (virFileReadAll(path, sizeof(buf), &buf) < 0) goto cleanup; - }
- if (fscanf(file, "%d", &ifindex) != 1) { + if (virStrToLong_i(buf, &c, 10, &ifindex) < 0 || *c != '\n') { virReportSystemError(errno, "%s", _("cannot determine macvtap's tap device " - "interface index")); + "interface index")); goto cleanup; }
- VIR_FORCE_FCLOSE(file); - - if (snprintf(tapname, sizeof(tapname), - "/dev/tap%d", ifindex) >= sizeof(tapname)) { - virReportSystemError(errno, - "%s", - _("internal buffer for tap device is too small")); + if (virAsprintf(&tapname, "/dev/tap%d", ifindex) < 0) goto cleanup; - }
while (1) { /* may need to wait for udev to be done */ @@ -292,7 +280,8 @@ int virNetDevMacVLanTapOpen(const char *ifname, ret = tapfd; cleanup: VIR_FREE(path); - VIR_FORCE_FCLOSE(file); + VIR_FREE(tapname); + VIR_FREE(buf); return ret; }
On 04.12.2015 17:02, Laine Stump wrote:
On 12/04/2015 07:30 AM, Michal Privoznik wrote:
There are few outdated things. Firstly, we don't need to undergo the torture of fopen, fscanf and fclose when we have nice wrapper over that: virFileReadAll. Secondly, we can use dynamically allocated buffer for the interface index.
Nothing against your changes to the existing function (ACK to that), but why is it reading sysfs for the ifindex? Why not just use virNetDevGetIndex(), as we do everywhere else in libvirt? (For that matter, I'm betting that the response message to the netlink request that creates the macvtap device will already contain the ifindex of the newly created device, but it would take more re-working of the code to carry that up from virNetDevMacVLanCreate() and over into virNetDevMacVLanTapOpen(), so likely not worth the small efficiency gain).
Oh, right! I'm switching to virNetDevGetIndex. Although frankly, I'm not much of a friend with netlink to know how to dig out anything from a response there. Michal
Hi, all, I am using libvirt to start a virtual machine, and I want to use TLS and VNC password. How can I build a virtual machine that connected with TLS and VNC password at the same time? who can give me an xml example? thks!
hi The following xml is enough for the virtual machine if you've configured vnc+tls env #virsh dumpxml guest_name -- <graphics type='vnc' port='-1' autoport='yes' passwd='123456'/> ----- Original Message -----
From: "c90006226" <opensource.cbc@aliyun.com> To: libvirt-list@redhat.com Sent: Monday, December 7, 2015 9:02:43 PM Subject: [libvirt] How can I build a virtual machine that connected with TLS and VNC password at the same time ?
Hi, all, I am using libvirt to start a virtual machine, and I want to use TLS and VNC password. How can I build a virtual machine that connected with TLS and VNC password at the same time? who can give me an xml example? thks!
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
hi I did not configure vnc+tls env, I use virsh domxml-from-native qemu.arg, buf there is no config in the xml about tls. Does it support configure tls with xml? 在 2015/12/8 12:45, Zhenfeng Wang 写道:
hi The following xml is enough for the virtual machine if you've configured vnc+tls env #virsh dumpxml guest_name -- <graphics type='vnc' port='-1' autoport='yes' passwd='123456'/>
----- Original Message -----
From: "c90006226" <opensource.cbc@aliyun.com> To: libvirt-list@redhat.com Sent: Monday, December 7, 2015 9:02:43 PM Subject: [libvirt] How can I build a virtual machine that connected with TLS and VNC password at the same time ?
Hi, all, I am using libvirt to start a virtual machine, and I want to use TLS and VNC password. How can I build a virtual machine that connected with TLS and VNC password at the same time? who can give me an xml example? thks!
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi Vnc didn't support config tls in the guest's xml, but could enable it in qemu.conf. Also we could see the tls enabled in the qemu command line while we start guest with tls enabled 1.configure vnctls env 2.start a guest with vnc password configured #virsh start rhel6 #virsh dumpxml rhel6 --security-info -- <graphics type='vnc' port='5900' autoport='yes' listen='0.0.0.0' passwd='123456'> <listen type='address' address='0.0.0.0'/> </graphics> #ps aux|grep qemu -- -vnc 0.0.0.0:0,password,tls,x509=/etc/pki/libvirt-vnc 3.if you're interesting about how to configure tlsvnc env, you can refer the following link http://wiki.libvirt.org/page/VNCTLSSetup ----- Original Message -----
From: "c90006226" <opensource.cbc@aliyun.com> To: "Zhenfeng Wang" <zhwang@redhat.com> Cc: libvirt-list@redhat.com, "Min Zhan" <mzhan@redhat.com> Sent: Tuesday, December 8, 2015 3:36:23 PM Subject: Re: [libvirt] How can I build a virtual machine that connected with TLS and VNC password at the same time ?
hi I did not configure vnc+tls env, I use virsh domxml-from-native qemu.arg, buf there is no config in the xml about tls. Does it support configure tls with xml?
在 2015/12/8 12:45, Zhenfeng Wang 写道:
hi The following xml is enough for the virtual machine if you've configured vnc+tls env #virsh dumpxml guest_name -- <graphics type='vnc' port='-1' autoport='yes' passwd='123456'/>
----- Original Message -----
From: "c90006226" <opensource.cbc@aliyun.com> To: libvirt-list@redhat.com Sent: Monday, December 7, 2015 9:02:43 PM Subject: [libvirt] How can I build a virtual machine that connected with TLS and VNC password at the same time ?
Hi, all, I am using libvirt to start a virtual machine, and I want to use TLS and VNC password. How can I build a virtual machine that connected with TLS and VNC password at the same time? who can give me an xml example? thks!
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
thanks ！ I had configured tlsvnc env， and can you tell me why libvirt do not support this？ Is there any special reason？ 在 2015/12/8 19:05, Zhenfeng Wang 写道:
Hi Vnc didn't support config tls in the guest's xml, but could enable it in qemu.conf. Also we could see the tls enabled in the qemu command line while we start guest with tls enabled
1.configure vnctls env 2.start a guest with vnc password configured #virsh start rhel6 #virsh dumpxml rhel6 --security-info -- <graphics type='vnc' port='5900' autoport='yes' listen='0.0.0.0' passwd='123456'> <listen type='address' address='0.0.0.0'/> </graphics>
#ps aux|grep qemu -- -vnc 0.0.0.0:0,password,tls,x509=/etc/pki/libvirt-vnc
3.if you're interesting about how to configure tlsvnc env, you can refer the following link http://wiki.libvirt.org/page/VNCTLSSetup
----- Original Message -----
From: "c90006226" <opensource.cbc@aliyun.com> To: "Zhenfeng Wang" <zhwang@redhat.com> Cc: libvirt-list@redhat.com, "Min Zhan" <mzhan@redhat.com> Sent: Tuesday, December 8, 2015 3:36:23 PM Subject: Re: [libvirt] How can I build a virtual machine that connected with TLS and VNC password at the same time ?
hi I did not configure vnc+tls env, I use virsh domxml-from-native qemu.arg, buf there is no config in the xml about tls. Does it support configure tls with xml?
在 2015/12/8 12:45, Zhenfeng Wang 写道:
hi The following xml is enough for the virtual machine if you've configured vnc+tls env #virsh dumpxml guest_name -- <graphics type='vnc' port='-1' autoport='yes' passwd='123456'/>
----- Original Message -----
From: "c90006226" <opensource.cbc@aliyun.com> To: libvirt-list@redhat.com Sent: Monday, December 7, 2015 9:02:43 PM Subject: [libvirt] How can I build a virtual machine that connected with TLS and VNC password at the same time ?
Hi, all, I am using libvirt to start a virtual machine, and I want to use TLS and VNC password. How can I build a virtual machine that connected with TLS and VNC password at the same time? who can give me an xml example? thks!
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Dec 08, 2015 at 09:40:50PM +0800, bccheng wrote:
thanks ！ I had configured tlsvnc env， and can you tell me why libvirt do not support this？ Is there any special reason？
Libvirt supports TLS just fine - you enable it in /etc/libvirt/qemu.conf though. We don't turn TLS on/off per VM - just at the host level for all VMs. Regards, 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 :|
So, for the multiqueue on macvtaps we are going to need to open the device multiple times. Currently, this is not supported. Rework the function, so that upper layers can be reworked too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 58 ++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index c1a5f0f..d20990b 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -226,15 +226,21 @@ int virNetDevMacVLanDelete(const char *ifname) /** * virNetDevMacVLanTapOpen: - * Open the macvtap's tap device. * @ifname: Name of the macvtap interface + * @tapfd: array of file descriptor return value for the new macvtap device + * @tapfdSize: number of file descriptors in @tapfd * @retries : Number of retries in case udev for example may need to be * waited for to create the tap chardev - * Returns negative value in case of error, the file descriptor otherwise. + * + * Open the macvtap's tap device, possibly multiple times if @tapfdSize > 1. + * + * Returns 0 on success, -1 otherwise. */ -static -int virNetDevMacVLanTapOpen(const char *ifname, - int retries) +static int +virNetDevMacVLanTapOpen(const char *ifname, + int *tapfd, + size_t tapfdSize, + int retries) { int ret = -1; char *path; @@ -242,7 +248,8 @@ int virNetDevMacVLanTapOpen(const char *ifname, char *tapname = NULL; char *buf = NULL; char *c; - int tapfd; + size_t i = 0; + int fd = -1; if (virNetDevSysfsFile(&path, ifname, "ifindex") < 0) return -1; @@ -260,25 +267,32 @@ int virNetDevMacVLanTapOpen(const char *ifname, if (virAsprintf(&tapname, "/dev/tap%d", ifindex) < 0) goto cleanup; - while (1) { - /* may need to wait for udev to be done */ - tapfd = open(tapname, O_RDWR); - if (tapfd < 0 && retries > 0) { - retries--; - usleep(20000); - continue; + for (i = 0; i < tapfdSize; i++) { + retry: + fd = open(tapname, O_RDWR); + if (fd < 0) { + if (retries > 0) { + /* may need to wait for udev to be done */ + retries--; + usleep(20000); + goto retry; + } + /* However, if haven't succeeded, quit. */ + virReportSystemError(errno, + _("cannot open macvtap tap device %s"), + tapname); + goto cleanup; } - break; + tapfd[i] = fd; } - if (tapfd < 0) { - virReportSystemError(errno, - _("cannot open macvtap tap device %s"), - tapname); - goto cleanup; - } - ret = tapfd; + ret = 0; + cleanup: + if (ret < 0) { + while (i--) + VIR_FORCE_CLOSE(tapfd[i]); + } VIR_FREE(path); VIR_FREE(tapname); VIR_FREE(buf); @@ -847,7 +861,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, } if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { - if ((rc = virNetDevMacVLanTapOpen(cr_ifname, 10)) < 0) + if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0) goto disassociate_exit; if (virNetDevMacVLanTapSetup(rc, vnet_hdr) < 0) { -- 2.4.10
On 12/04/2015 07:30 AM, Michal Privoznik wrote:
So, for the multiqueue on macvtaps we are going to need to open
s/So, for the multiqueue/For multiqueue support/
the device multiple times. Currently, this is not supported. Rework the function, so that upper layers can be reworked too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 58 ++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index c1a5f0f..d20990b 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -226,15 +226,21 @@ int virNetDevMacVLanDelete(const char *ifname)
/** * virNetDevMacVLanTapOpen: - * Open the macvtap's tap device. * @ifname: Name of the macvtap interface + * @tapfd: array of file descriptor return value for the new macvtap device + * @tapfdSize: number of file descriptors in @tapfd * @retries : Number of retries in case udev for example may need to be * waited for to create the tap chardev - * Returns negative value in case of error, the file descriptor otherwise. + * + * Open the macvtap's tap device, possibly multiple times if @tapfdSize > 1. + * + * Returns 0 on success, -1 otherwise. */ -static -int virNetDevMacVLanTapOpen(const char *ifname, - int retries) +static int +virNetDevMacVLanTapOpen(const char *ifname, + int *tapfd, + size_t tapfdSize, + int retries) { int ret = -1; char *path; @@ -242,7 +248,8 @@ int virNetDevMacVLanTapOpen(const char *ifname, char *tapname = NULL; char *buf = NULL; char *c; - int tapfd; + size_t i = 0; + int fd = -1;
if (virNetDevSysfsFile(&path, ifname, "ifindex") < 0) return -1; @@ -260,25 +267,32 @@ int virNetDevMacVLanTapOpen(const char *ifname, if (virAsprintf(&tapname, "/dev/tap%d", ifindex) < 0) goto cleanup;
- while (1) { - /* may need to wait for udev to be done */ - tapfd = open(tapname, O_RDWR); - if (tapfd < 0 && retries > 0) { - retries--; - usleep(20000); - continue; + for (i = 0; i < tapfdSize; i++) { + retry: + fd = open(tapname, O_RDWR); + if (fd < 0) { + if (retries > 0) { + /* may need to wait for udev to be done */ + retries--; + usleep(20000); + goto retry; + } + /* However, if haven't succeeded, quit. */ + virReportSystemError(errno, + _("cannot open macvtap tap device %s"), + tapname); + goto cleanup; } - break; + tapfd[i] = fd; }
Although libvirt very commonly uses goto essentially for "exception handling in C", and that does make the code quite a bit cleaner, I still have an aversion to goto created by the graders in my very first Pascal class eons ago - if you had a goto, it wasn't accepted unless you explained how you would have written the code without it, and then they *still* took half a mark from your grade. This is why when I see the above, I would prefer to write something like this: for (i = 0; i < tapfdSize; i++) { int fd = -1; while (fd < 0) { if ((fd = open(tapname, O_RDWR)) >= 0) { tapfd[i] = fd; } else if (retries-- > 0) { usleep(20000); } else { virReportSystemError(errno, _("cannot open macvtap tap device %s"), tapname); goto cleanup; } } } Note that this has the effect of allowing some retries to count when opening the first fd, and other retries to count when opening the others (similar to your loop, btw). Nothing wrong with this, since the retries are there to wait for udev to complete creation of the tap device (so once it's done, we'll never need to wait for it again), just pointing it out. ACK. It's up to you whether you want to use the loop with an extra label or the one without; I'm not going to insist on it (but my grader from [redacted to avoid shocking anyone] years ago would frown at me in my sleep if I didn't mention it :-))
- if (tapfd < 0) { - virReportSystemError(errno, - _("cannot open macvtap tap device %s"), - tapname); - goto cleanup; - } - ret = tapfd; + ret = 0; + cleanup: + if (ret < 0) { + while (i--) + VIR_FORCE_CLOSE(tapfd[i]); + } VIR_FREE(path); VIR_FREE(tapname); VIR_FREE(buf); @@ -847,7 +861,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, }
if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { - if ((rc = virNetDevMacVLanTapOpen(cr_ifname, 10)) < 0) + if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0) goto disassociate_exit;
if (virNetDevMacVLanTapSetup(rc, vnet_hdr) < 0) {
So, for the multiqueue on macvtaps we are going to need to open the device multiple times. Currently, this is not supported. Rework the function, so that upper layers can be reworked too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 66 ++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index d20990b..f7a7d72 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -302,10 +302,9 @@ virNetDevMacVLanTapOpen(const char *ifname, /** * virNetDevMacVLanTapSetup: - * @tapfd: file descriptor of the macvtap tap - * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it - * - * Returns 0 on success, -1 in case of fatal error, error code otherwise. + * @tapfd: array of file descriptors of the macvtap tap + * @tapfdSize: number of file descriptors in @tapfd + * @vnet_hdr: whether to enable or disable IFF_VNET_HDR * * Turn the IFF_VNET_HDR flag, if requested and available, make sure * it's off in the other cases. @@ -313,47 +312,52 @@ virNetDevMacVLanTapOpen(const char *ifname, * be turned off for some reason. This is reported with -1. Other fatal * error is not being able to read the interface flags. In that case the * macvtap device should not be used. + * + * Returns 0 on success, -1 in case of fatal error, error code otherwise. */ static int -virNetDevMacVLanTapSetup(int tapfd, int vnet_hdr) +virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr) { unsigned int features; struct ifreq ifreq; short new_flags = 0; int rc_on_fail = 0; const char *errmsg = NULL; + size_t i; - memset(&ifreq, 0, sizeof(ifreq)); + for (i = 0; i < tapfdSize; i++) { + memset(&ifreq, 0, sizeof(ifreq)); - if (ioctl(tapfd, TUNGETIFF, &ifreq) < 0) { - virReportSystemError(errno, "%s", - _("cannot get interface flags on macvtap tap")); - return -1; - } - - new_flags = ifreq.ifr_flags; - - if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) { - new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR; - rc_on_fail = -1; - errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap"); - } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) { - if (ioctl(tapfd, TUNGETFEATURES, &features) < 0) { + if (ioctl(tapfd[i], TUNGETIFF, &ifreq) < 0) { virReportSystemError(errno, "%s", - _("cannot get feature flags on macvtap tap")); + _("cannot get interface flags on macvtap tap")); return -1; } - if ((features & IFF_VNET_HDR)) { - new_flags = ifreq.ifr_flags | IFF_VNET_HDR; - errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap"); + + new_flags = ifreq.ifr_flags; + + if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) { + new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR; + rc_on_fail = -1; + errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap"); + } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) { + if (ioctl(tapfd[i], TUNGETFEATURES, &features) < 0) { + virReportSystemError(errno, "%s", + _("cannot get feature flags on macvtap tap")); + return -1; + } + if ((features & IFF_VNET_HDR)) { + new_flags = ifreq.ifr_flags | IFF_VNET_HDR; + errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap"); + } } - } - if (new_flags != ifreq.ifr_flags) { - ifreq.ifr_flags = new_flags; - if (ioctl(tapfd, TUNSETIFF, &ifreq) < 0) { - virReportSystemError(errno, "%s", errmsg); - return rc_on_fail; + if (new_flags != ifreq.ifr_flags) { + ifreq.ifr_flags = new_flags; + if (ioctl(tapfd[i], TUNSETIFF, &ifreq) < 0) { + virReportSystemError(errno, "%s", errmsg); + return rc_on_fail; + } } } @@ -864,7 +868,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0) goto disassociate_exit; - if (virNetDevMacVLanTapSetup(rc, vnet_hdr) < 0) { + if (virNetDevMacVLanTapSetup(&rc, 1, vnet_hdr) < 0) { VIR_FORCE_CLOSE(rc); /* sets rc to -1 */ goto disassociate_exit; } -- 2.4.10
On 12/04/2015 07:31 AM, Michal Privoznik wrote:
So, for the multiqueue on macvtaps we are going to need to open the device multiple times. Currently, this is not supported. Rework the function, so that upper layers can be reworked too.
Either merge 3 & 4 together, or change the log message (I'm okay with multiple functions at the same level being changed in the same patch).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 66 ++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 31 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index d20990b..f7a7d72 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -302,10 +302,9 @@ virNetDevMacVLanTapOpen(const char *ifname,
/** * virNetDevMacVLanTapSetup: - * @tapfd: file descriptor of the macvtap tap - * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it - * - * Returns 0 on success, -1 in case of fatal error, error code otherwise. + * @tapfd: array of file descriptors of the macvtap tap + * @tapfdSize: number of file descriptors in @tapfd + * @vnet_hdr: whether to enable or disable IFF_VNET_HDR * * Turn the IFF_VNET_HDR flag, if requested and available, make sure * it's off in the other cases. @@ -313,47 +312,52 @@ virNetDevMacVLanTapOpen(const char *ifname, * be turned off for some reason. This is reported with -1. Other fatal * error is not being able to read the interface flags. In that case the * macvtap device should not be used. + *
Comparing this to the VNET_HDR stuff for tap devices, I see that in that case lack of support for VNET_HDR isn't fatal - it simply isn't set. My guess is that VNET_HDR support was probably added prior to macvtap, so that it's always there if we're doing macvtap, but I don't know for sure. (I'm just wondering if maybe there's some way code could be shared between the two to reduce maintenance). The other difference is that IFF_MULTI_QUEUE is set on each fd for a tap device. Is this also needed for macvtap?
+ * Returns 0 on success, -1 in case of fatal error, error code otherwise. */ static int -virNetDevMacVLanTapSetup(int tapfd, int vnet_hdr) +virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr) { unsigned int features; struct ifreq ifreq; short new_flags = 0; int rc_on_fail = 0; const char *errmsg = NULL; + size_t i;
- memset(&ifreq, 0, sizeof(ifreq)); + for (i = 0; i < tapfdSize; i++) { + memset(&ifreq, 0, sizeof(ifreq));
- if (ioctl(tapfd, TUNGETIFF, &ifreq) < 0) { - virReportSystemError(errno, "%s", - _("cannot get interface flags on macvtap tap")); - return -1; - } - - new_flags = ifreq.ifr_flags; - - if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) { - new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR; - rc_on_fail = -1; - errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap"); - } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) { - if (ioctl(tapfd, TUNGETFEATURES, &features) < 0) { + if (ioctl(tapfd[i], TUNGETIFF, &ifreq) < 0) { virReportSystemError(errno, "%s", - _("cannot get feature flags on macvtap tap")); + _("cannot get interface flags on macvtap tap")); return -1; } - if ((features & IFF_VNET_HDR)) { - new_flags = ifreq.ifr_flags | IFF_VNET_HDR; - errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap"); + + new_flags = ifreq.ifr_flags; + + if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) { + new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR; + rc_on_fail = -1; + errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap"); + } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) { + if (ioctl(tapfd[i], TUNGETFEATURES, &features) < 0) { + virReportSystemError(errno, "%s", + _("cannot get feature flags on macvtap tap")); + return -1; + } + if ((features & IFF_VNET_HDR)) { + new_flags = ifreq.ifr_flags | IFF_VNET_HDR; + errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap"); + } } - }
- if (new_flags != ifreq.ifr_flags) { - ifreq.ifr_flags = new_flags; - if (ioctl(tapfd, TUNSETIFF, &ifreq) < 0) { - virReportSystemError(errno, "%s", errmsg); - return rc_on_fail; + if (new_flags != ifreq.ifr_flags) { + ifreq.ifr_flags = new_flags; + if (ioctl(tapfd[i], TUNSETIFF, &ifreq) < 0) { + virReportSystemError(errno, "%s", errmsg); + return rc_on_fail; + } } }
@@ -864,7 +868,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0) goto disassociate_exit;
- if (virNetDevMacVLanTapSetup(rc, vnet_hdr) < 0) { + if (virNetDevMacVLanTapSetup(&rc, 1, vnet_hdr) < 0) { VIR_FORCE_CLOSE(rc); /* sets rc to -1 */ goto disassociate_exit; }
On 04.12.2015 18:43, Laine Stump wrote:
On 12/04/2015 07:31 AM, Michal Privoznik wrote:
So, for the multiqueue on macvtaps we are going to need to open the device multiple times. Currently, this is not supported. Rework the function, so that upper layers can be reworked too.
Either merge 3 & 4 together, or change the log message (I'm okay with multiple functions at the same level being changed in the same patch).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 66 ++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 31 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index d20990b..f7a7d72 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -302,10 +302,9 @@ virNetDevMacVLanTapOpen(const char *ifname, /** * virNetDevMacVLanTapSetup: - * @tapfd: file descriptor of the macvtap tap - * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it - * - * Returns 0 on success, -1 in case of fatal error, error code otherwise. + * @tapfd: array of file descriptors of the macvtap tap + * @tapfdSize: number of file descriptors in @tapfd + * @vnet_hdr: whether to enable or disable IFF_VNET_HDR * * Turn the IFF_VNET_HDR flag, if requested and available, make sure * it's off in the other cases. @@ -313,47 +312,52 @@ virNetDevMacVLanTapOpen(const char *ifname, * be turned off for some reason. This is reported with -1. Other fatal * error is not being able to read the interface flags. In that case the * macvtap device should not be used. + *
Comparing this to the VNET_HDR stuff for tap devices, I see that in that case lack of support for VNET_HDR isn't fatal - it simply isn't set. My guess is that VNET_HDR support was probably added prior to macvtap, so that it's always there if we're doing macvtap, but I don't know for sure. (I'm just wondering if maybe there's some way code could be shared between the two to reduce maintenance).
The other difference is that IFF_MULTI_QUEUE is set on each fd for a tap device. Is this also needed for macvtap?
Good point. Honestly, I don't know. I have it working locally even without the flag. Let me ask the kvm guy that implemented the feature there and get back. Michal
So, for the multiqueue on macvtaps we are going to need to open the device multiple times. Currently, this is not supported. Rework the function, so that upper layers can be reworked too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_process.c | 1 + src/qemu/qemu_command.c | 21 ++++++++++++--------- src/util/virnetdevmacvlan.c | 18 +++++++++++++----- src/util/virnetdevmacvlan.h | 4 +++- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0ada6e4..f7e2b81 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -349,6 +349,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, &res_ifname, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, cfg->stateDir, + NULL, 0, macvlan_create_flags) < 0) goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 55809e9..d1ef4ab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -230,15 +230,18 @@ qemuPhysIfaceConnect(virDomainDefPtr def, if (net->model && STREQ(net->model, "virtio")) macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR; - rc = virNetDevMacVLanCreateWithVPortProfile( - net->ifname, &net->mac, - virDomainNetGetActualDirectDev(net), - virDomainNetGetActualDirectMode(net), - def->uuid, - virDomainNetGetActualVirtPortProfile(net), - &res_ifname, - vmop, cfg->stateDir, - macvlan_create_flags); + if (virNetDevMacVLanCreateWithVPortProfile(net->ifname, + &net->mac, + virDomainNetGetActualDirectDev(net), + virDomainNetGetActualDirectMode(net), + def->uuid, + virDomainNetGetActualVirtPortProfile(net), + &res_ifname, + vmop, cfg->stateDir, + &rc, 1, + macvlan_create_flags) < 0) + return -1; + if (rc >= 0) { virDomainAuditNetDevice(def, net, res_ifname, true); VIR_FREE(net->ifname); diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index f7a7d72..244e01a 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -739,11 +739,15 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, * @res_ifname: Pointer to a string pointer where the actual name of the * interface will be stored into if everything succeeded. It is up * to the caller to free the string. + * @tapfd: array of file descriptor return value for the new tap device + * @tapfdSize: number of file descriptors in @tapfd * @flags: OR of virNetDevMacVLanCreateFlags. * - * Returns file descriptor of the tap device in case of success with - * @flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP, otherwise returns 0; returns - * -1 on error. + * Creates a macvlan device. Optionally, if flags & + * VIR_NETDEV_MACVLAN_CREATE_WITH_TAP is set, @tapfd is populated with FDs of + * tap devices up to @tapfdSize. + * + * Return 0 on success, -1 on error. */ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const virMacAddr *macaddress, @@ -754,6 +758,8 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, char **res_ifname, virNetDevVPortProfileOp vmOp, char *stateDir, + int *tapfd, + size_t tapfdSize, unsigned int flags) { const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ? @@ -865,10 +871,10 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, } if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { - if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0) + if (virNetDevMacVLanTapOpen(cr_ifname, tapfd, tapfdSize, 10) < 0) goto disassociate_exit; - if (virNetDevMacVLanTapSetup(&rc, 1, vnet_hdr) < 0) { + if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr) < 0) { VIR_FORCE_CLOSE(rc); /* sets rc to -1 */ goto disassociate_exit; } @@ -1028,6 +1034,8 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname ATTRIBUTE_UNUSED, char **res_ifname ATTRIBUTE_UNUSED, virNetDevVPortProfileOp vmop ATTRIBUTE_UNUSED, char *stateDir ATTRIBUTE_UNUSED, + int *tapfd ATTRIBUTE_UNUSED, + size_t tapfdSize ATTRIBUTE_UNUSED, unsigned int unused_flags ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index b04fc8c..08ccdd2 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -71,9 +71,11 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname, char **res_ifname, virNetDevVPortProfileOp vmop, char *stateDir, + int *tapfd, + size_t tapfdSize, unsigned int flags) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6) - ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(10) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(12) ATTRIBUTE_RETURN_CHECK; int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, const virMacAddr *macaddress, -- 2.4.10
On 12/04/2015 07:31 AM, Michal Privoznik wrote:
So, for the multiqueue on macvtaps we are going to need to open the device multiple times. Currently, this is not supported. Rework the function, so that upper layers can be reworked too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
I don't see any extra value (beyond review-time :-) in having this patch separated from the previous 2. I think they could just as well be combined into a single patch, leaving only the top-level enablement in the qemu driver as a separate patch (as well as patches 1 and 2, since they aren't purely adding in the multi-fd support.) ACK for what's here, however it gets pushed.
src/lxc/lxc_process.c | 1 + src/qemu/qemu_command.c | 21 ++++++++++++--------- src/util/virnetdevmacvlan.c | 18 +++++++++++++----- src/util/virnetdevmacvlan.h | 4 +++- 4 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0ada6e4..f7e2b81 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -349,6 +349,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, &res_ifname, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, cfg->stateDir, + NULL, 0, macvlan_create_flags) < 0) goto cleanup;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 55809e9..d1ef4ab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -230,15 +230,18 @@ qemuPhysIfaceConnect(virDomainDefPtr def, if (net->model && STREQ(net->model, "virtio")) macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR;
- rc = virNetDevMacVLanCreateWithVPortProfile( - net->ifname, &net->mac, - virDomainNetGetActualDirectDev(net), - virDomainNetGetActualDirectMode(net), - def->uuid, - virDomainNetGetActualVirtPortProfile(net), - &res_ifname, - vmop, cfg->stateDir, - macvlan_create_flags); + if (virNetDevMacVLanCreateWithVPortProfile(net->ifname, + &net->mac, + virDomainNetGetActualDirectDev(net), + virDomainNetGetActualDirectMode(net), + def->uuid, + virDomainNetGetActualVirtPortProfile(net), + &res_ifname, + vmop, cfg->stateDir, + &rc, 1, + macvlan_create_flags) < 0) + return -1; + if (rc >= 0) { virDomainAuditNetDevice(def, net, res_ifname, true); VIR_FREE(net->ifname); diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index f7a7d72..244e01a 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -739,11 +739,15 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, * @res_ifname: Pointer to a string pointer where the actual name of the * interface will be stored into if everything succeeded. It is up * to the caller to free the string. + * @tapfd: array of file descriptor return value for the new tap device + * @tapfdSize: number of file descriptors in @tapfd * @flags: OR of virNetDevMacVLanCreateFlags. * - * Returns file descriptor of the tap device in case of success with - * @flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP, otherwise returns 0; returns - * -1 on error. + * Creates a macvlan device. Optionally, if flags & + * VIR_NETDEV_MACVLAN_CREATE_WITH_TAP is set, @tapfd is populated with FDs of + * tap devices up to @tapfdSize. + * + * Return 0 on success, -1 on error. */ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const virMacAddr *macaddress, @@ -754,6 +758,8 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, char **res_ifname, virNetDevVPortProfileOp vmOp, char *stateDir, + int *tapfd, + size_t tapfdSize, unsigned int flags) { const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ? @@ -865,10 +871,10 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, }
if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { - if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0) + if (virNetDevMacVLanTapOpen(cr_ifname, tapfd, tapfdSize, 10) < 0) goto disassociate_exit;
- if (virNetDevMacVLanTapSetup(&rc, 1, vnet_hdr) < 0) { + if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr) < 0) { VIR_FORCE_CLOSE(rc); /* sets rc to -1 */ goto disassociate_exit; } @@ -1028,6 +1034,8 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname ATTRIBUTE_UNUSED, char **res_ifname ATTRIBUTE_UNUSED, virNetDevVPortProfileOp vmop ATTRIBUTE_UNUSED, char *stateDir ATTRIBUTE_UNUSED, + int *tapfd ATTRIBUTE_UNUSED, + size_t tapfdSize ATTRIBUTE_UNUSED, unsigned int unused_flags ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index b04fc8c..08ccdd2 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -71,9 +71,11 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname, char **res_ifname, virNetDevVPortProfileOp vmop, char *stateDir, + int *tapfd, + size_t tapfdSize, unsigned int flags) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6) - ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(10) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(12) ATTRIBUTE_RETURN_CHECK;
int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, const virMacAddr *macaddress,
https://bugzilla.redhat.com/show_bug.cgi?id=1240439 Ta-da! Now that we know how to open a macvtap device multiple times, we can finally enable the multiqueue feature. Everything else is already prepared (e.g. command line generation) from the previous iteration where the feature was implemented for TUN/TAP devices. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 41 ++++++++++++++++++++++++++--------------- src/qemu/qemu_command.h | 2 ++ src/qemu/qemu_hotplug.c | 16 +++++++++------- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d1ef4ab..821204e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -212,17 +212,21 @@ qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) * @def: the definition of the VM (needed by 802.1Qbh and audit) * @driver: pointer to the driver instance * @net: pointer to the VM's interface description with direct device type + * @tapfd: array of file descriptor return value for the new device + * @tapfdSize: number of file descriptors in @tapfd * @vmop: VM operation type * - * Returns a filedescriptor on success or -1 in case of error. + * Returns 0 on success or -1 in case of error. */ int qemuPhysIfaceConnect(virDomainDefPtr def, virQEMUDriverPtr driver, virDomainNetDefPtr net, + int *tapfd, + size_t tapfdSize, virNetDevVPortProfileOp vmop) { - int rc; + int ret = -1; char *res_ifname = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP; @@ -238,18 +242,18 @@ qemuPhysIfaceConnect(virDomainDefPtr def, virDomainNetGetActualVirtPortProfile(net), &res_ifname, vmop, cfg->stateDir, - &rc, 1, + tapfd, tapfdSize, macvlan_create_flags) < 0) - return -1; + goto cleanup; - if (rc >= 0) { - virDomainAuditNetDevice(def, net, res_ifname, true); - VIR_FREE(net->ifname); - net->ifname = res_ifname; - } + virDomainAuditNetDevice(def, net, res_ifname, true); + VIR_FREE(net->ifname); + net->ifname = res_ifname; + ret = 0; + cleanup: virObjectUnref(cfg); - return rc; + return ret; } @@ -8746,7 +8750,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, /* Currently nothing besides TAP devices supports multiqueue. */ if (net->driver.virtio.queues > 0 && !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_DIRECT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Multiqueue network is not supported for: %s"), virDomainNetTypeToString(actualType)); @@ -8791,11 +8796,17 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, tapfd, &tapfdSize) < 0) goto cleanup; } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(tapfdName) < 0) + tapfdSize = net->driver.virtio.queues; + if (!tapfdSize) + tapfdSize = 1; + + if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 || + VIR_ALLOC_N(tapfdName, tapfdSize) < 0) goto cleanup; - tapfdSize = 1; - tapfd[0] = qemuPhysIfaceConnect(def, driver, net, vmop); - if (tapfd[0] < 0) + + memset(tapfd, -1, tapfdSize * sizeof(tapfd[0])); + + if (qemuPhysIfaceConnect(def, driver, net, tapfd, tapfdSize, vmop) < 0) goto cleanup; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index bebdd27..f0d6900 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -232,6 +232,8 @@ int qemuNetworkIfaceConnect(virDomainDefPtr def, int qemuPhysIfaceConnect(virDomainDefPtr def, virQEMUDriverPtr driver, virDomainNetDefPtr net, + int *tapfd, + size_t tapfdSize, virNetDevVPortProfileOp vmop); int qemuOpenVhostNet(virDomainDefPtr def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8804d3d..1a3b278 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -947,15 +947,17 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) goto cleanup; } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - tapfdSize = vhostfdSize = 1; - if (VIR_ALLOC(tapfd) < 0) + tapfdSize = vhostfdSize = net->driver.virtio.queues; + if (!tapfdSize) + tapfdSize = vhostfdSize = 1; + if (VIR_ALLOC_N(tapfd, tapfdSize) < 0) goto cleanup; - *tapfd = -1; - if (VIR_ALLOC(vhostfd) < 0) + memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); + if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0) goto cleanup; - *vhostfd = -1; - if ((tapfd[0] = qemuPhysIfaceConnect(vm->def, driver, net, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) < 0) + memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); + if (qemuPhysIfaceConnect(vm->def, driver, net, tapfd, tapfdSize, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) goto cleanup; iface_connected = true; if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) -- 2.4.10
On 12/04/2015 07:31 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1240439
Ta-da! Now that we know how to open a macvtap device multiple times, we can finally enable the multiqueue feature. Everything else is already prepared (e.g. command line generation) from the previous iteration where the feature was implemented for TUN/TAP devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
ACK.
src/qemu/qemu_command.c | 41 ++++++++++++++++++++++++++--------------- src/qemu/qemu_command.h | 2 ++ src/qemu/qemu_hotplug.c | 16 +++++++++------- 3 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d1ef4ab..821204e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -212,17 +212,21 @@ qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) * @def: the definition of the VM (needed by 802.1Qbh and audit) * @driver: pointer to the driver instance * @net: pointer to the VM's interface description with direct device type + * @tapfd: array of file descriptor return value for the new device + * @tapfdSize: number of file descriptors in @tapfd * @vmop: VM operation type * - * Returns a filedescriptor on success or -1 in case of error. + * Returns 0 on success or -1 in case of error. */ int qemuPhysIfaceConnect(virDomainDefPtr def, virQEMUDriverPtr driver, virDomainNetDefPtr net, + int *tapfd, + size_t tapfdSize, virNetDevVPortProfileOp vmop) { - int rc; + int ret = -1; char *res_ifname = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP; @@ -238,18 +242,18 @@ qemuPhysIfaceConnect(virDomainDefPtr def, virDomainNetGetActualVirtPortProfile(net), &res_ifname, vmop, cfg->stateDir, - &rc, 1, + tapfd, tapfdSize, macvlan_create_flags) < 0) - return -1; + goto cleanup;
- if (rc >= 0) { - virDomainAuditNetDevice(def, net, res_ifname, true); - VIR_FREE(net->ifname); - net->ifname = res_ifname; - } + virDomainAuditNetDevice(def, net, res_ifname, true); + VIR_FREE(net->ifname); + net->ifname = res_ifname; + ret = 0;
+ cleanup: virObjectUnref(cfg); - return rc; + return ret; }
@@ -8746,7 +8750,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, /* Currently nothing besides TAP devices supports multiqueue. */ if (net->driver.virtio.queues > 0 && !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_DIRECT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Multiqueue network is not supported for: %s"), virDomainNetTypeToString(actualType)); @@ -8791,11 +8796,17 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, tapfd, &tapfdSize) < 0) goto cleanup; } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(tapfdName) < 0) + tapfdSize = net->driver.virtio.queues; + if (!tapfdSize) + tapfdSize = 1; + + if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 || + VIR_ALLOC_N(tapfdName, tapfdSize) < 0) goto cleanup; - tapfdSize = 1; - tapfd[0] = qemuPhysIfaceConnect(def, driver, net, vmop); - if (tapfd[0] < 0) + + memset(tapfd, -1, tapfdSize * sizeof(tapfd[0])); + + if (qemuPhysIfaceConnect(def, driver, net, tapfd, tapfdSize, vmop) < 0) goto cleanup; }
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index bebdd27..f0d6900 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -232,6 +232,8 @@ int qemuNetworkIfaceConnect(virDomainDefPtr def, int qemuPhysIfaceConnect(virDomainDefPtr def, virQEMUDriverPtr driver, virDomainNetDefPtr net, + int *tapfd, + size_t tapfdSize, virNetDevVPortProfileOp vmop);
int qemuOpenVhostNet(virDomainDefPtr def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8804d3d..1a3b278 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -947,15 +947,17 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) goto cleanup; } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - tapfdSize = vhostfdSize = 1; - if (VIR_ALLOC(tapfd) < 0) + tapfdSize = vhostfdSize = net->driver.virtio.queues; + if (!tapfdSize) + tapfdSize = vhostfdSize = 1; + if (VIR_ALLOC_N(tapfd, tapfdSize) < 0) goto cleanup; - *tapfd = -1; - if (VIR_ALLOC(vhostfd) < 0) + memset(tapfd, -1, sizeof(*tapfd) * tapfdSize); + if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0) goto cleanup; - *vhostfd = -1; - if ((tapfd[0] = qemuPhysIfaceConnect(vm->def, driver, net, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) < 0) + memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); + if (qemuPhysIfaceConnect(vm->def, driver, net, tapfd, tapfdSize, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) goto cleanup; iface_connected = true; if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0)
participants (6)
-
bccheng -
c90006226
-
Daniel P. Berrange -
Laine Stump -
Michal Privoznik -
Zhenfeng Wang