[libvirt] [PATCH v2 0/3] improve virsh attach-interface
new in v2: - removed netdev name as an option for --source parameter - removed --driver parameter, the default is vfio for new qemu and that's good enough Pavel Hrdina (3): virsh-nodedev: makes struct and functions for NodeDevice list available virsh-domain: update attach-interface to support type=hostdev virsh.pod: update and improve a attach-interface section tools/virsh-domain.c | 34 +++++++++++++++++++-- tools/virsh-nodedev.c | 16 ++++------ tools/virsh-nodedev.h | 11 +++++++ tools/virsh.pod | 82 +++++++++++++++++++++++++++++++-------------------- 4 files changed, 98 insertions(+), 45 deletions(-) -- 2.6.2
Next patch will use those function to collect NodeDevice list and find a specific device. Make functions virshNodeDeviceListCollect() and virshNodeDeviceListFree() together with struct virshNodeDeviceList available to reuse existing code. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh-nodedev.c | 16 +++++----------- tools/virsh-nodedev.h | 11 +++++++++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index cc359e2..26f2c7b 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -194,13 +194,7 @@ virshNodeDeviceSorter(const void *a, const void *b) virNodeDeviceGetName(*nb)); } -struct virshNodeDeviceList { - virNodeDevicePtr *devices; - size_t ndevices; -}; -typedef struct virshNodeDeviceList *virshNodeDeviceListPtr; - -static void +void virshNodeDeviceListFree(virshNodeDeviceListPtr list) { size_t i; @@ -215,11 +209,11 @@ virshNodeDeviceListFree(virshNodeDeviceListPtr list) VIR_FREE(list); } -static virshNodeDeviceListPtr +virshNodeDeviceListPtr virshNodeDeviceListCollect(vshControl *ctl, - char **capnames, - int ncapnames, - unsigned int flags) + char **capnames, + int ncapnames, + unsigned int flags) { virshNodeDeviceListPtr list = vshMalloc(ctl, sizeof(*list)); size_t i; diff --git a/tools/virsh-nodedev.h b/tools/virsh-nodedev.h index c64f7df..1d2337b 100644 --- a/tools/virsh-nodedev.h +++ b/tools/virsh-nodedev.h @@ -30,4 +30,15 @@ extern const vshCmdDef nodedevCmds[]; +struct virshNodeDeviceList { + virNodeDevicePtr *devices; + size_t ndevices; +}; +typedef struct virshNodeDeviceList *virshNodeDeviceListPtr; + +virshNodeDeviceListPtr virshNodeDeviceListCollect(vshControl *ctl, + char **capnames, + int ncapnames, + unsigned int flags); +void virshNodeDeviceListFree(virshNodeDeviceListPtr list); #endif /* VIRSH_NODEDEV_H */ -- 2.6.2
$SUBJ: Expose virshNodeDeviceList{Collect|Free} and virshNodeDeviceList struct On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Next patch will use those function to collect NodeDevice list and find a specific device. Make functions virshNodeDeviceListCollect() and virshNodeDeviceListFree() together with struct virshNodeDeviceList available to reuse existing code.
Exposing virshNodeDeviceListCollect, virshNodeDeviceListFree, and virshNodeDeviceList allows the data returned to be available to other virsh API's that may need them in the future.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh-nodedev.c | 16 +++++----------- tools/virsh-nodedev.h | 11 +++++++++++ 2 files changed, 16 insertions(+), 11 deletions(-)
OK - all that said, but your future patches don't use these functions, so is there really any use for this patch yet? It seems your 2/3 has removed what was in the 3/4 in your prior series related to calling virshNodeDeviceListCollect (and noted in your cover letter as being removed). I don't oppose the change, but it doesn't seem necessary. John
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index cc359e2..26f2c7b 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -194,13 +194,7 @@ virshNodeDeviceSorter(const void *a, const void *b) virNodeDeviceGetName(*nb)); }
-struct virshNodeDeviceList { - virNodeDevicePtr *devices; - size_t ndevices; -}; -typedef struct virshNodeDeviceList *virshNodeDeviceListPtr; - -static void +void virshNodeDeviceListFree(virshNodeDeviceListPtr list) { size_t i; @@ -215,11 +209,11 @@ virshNodeDeviceListFree(virshNodeDeviceListPtr list) VIR_FREE(list); }
-static virshNodeDeviceListPtr +virshNodeDeviceListPtr virshNodeDeviceListCollect(vshControl *ctl, - char **capnames, - int ncapnames, - unsigned int flags) + char **capnames, + int ncapnames, + unsigned int flags) { virshNodeDeviceListPtr list = vshMalloc(ctl, sizeof(*list)); size_t i; diff --git a/tools/virsh-nodedev.h b/tools/virsh-nodedev.h index c64f7df..1d2337b 100644 --- a/tools/virsh-nodedev.h +++ b/tools/virsh-nodedev.h @@ -30,4 +30,15 @@
extern const vshCmdDef nodedevCmds[];
+struct virshNodeDeviceList { + virNodeDevicePtr *devices; + size_t ndevices; +}; +typedef struct virshNodeDeviceList *virshNodeDeviceListPtr; + +virshNodeDeviceListPtr virshNodeDeviceListCollect(vshControl *ctl, + char **capnames, + int ncapnames, + unsigned int flags); +void virshNodeDeviceListFree(virshNodeDeviceListPtr list); #endif /* VIRSH_NODEDEV_H */
On Tue, Oct 27, 2015 at 06:10:35PM -0400, John Ferlan wrote:
$SUBJ: Expose virshNodeDeviceList{Collect|Free} and virshNodeDeviceList struct
On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Next patch will use those function to collect NodeDevice list and find a specific device. Make functions virshNodeDeviceListCollect() and virshNodeDeviceListFree() together with struct virshNodeDeviceList available to reuse existing code.
Exposing virshNodeDeviceListCollect, virshNodeDeviceListFree, and virshNodeDeviceList allows the data returned to be available to other virsh API's that may need them in the future.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh-nodedev.c | 16 +++++----------- tools/virsh-nodedev.h | 11 +++++++++++ 2 files changed, 16 insertions(+), 11 deletions(-)
OK - all that said, but your future patches don't use these functions, so is there really any use for this patch yet? It seems your 2/3 has removed what was in the 3/4 in your prior series related to calling virshNodeDeviceListCollect (and noted in your cover letter as being removed).
I don't oppose the change, but it doesn't seem necessary.
Thanks, I don't know why I didn't remove this patch from series, because it's not required anymore. I'm not pushing this one. Thanks, Pavel
Adding this feature will allow users to easily attach a hostdev network interface using PCI passthrough. The interface can be attached using --type=hostdev and PCI address or as --source. This command also allows you to tell, whether the interface should be managed. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=997561 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh-domain.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12e85e3..bd00785 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -56,6 +56,7 @@ #include "virtime.h" #include "virtypedparam.h" #include "virxml.h" +#include "virsh-nodedev.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -866,6 +867,10 @@ static const vshCmdOptDef opts_attach_interface[] = { .type = VSH_OT_BOOL, .help = N_("print XML document rather than attach the interface") }, + {.name = "managed", + .type = VSH_OT_BOOL, + .help = N_("libvirt will automatically detach/attach the device from/to host") + }, {.name = NULL} }; @@ -931,6 +936,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool persistent = vshCommandOptBool(cmd, "persistent"); + bool managed = vshCommandOptBool(cmd, "managed"); VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); @@ -983,7 +989,12 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) } /* Make XML of interface */ - virBufferAsprintf(&buf, "<interface type='%s'>\n", type); + virBufferAsprintf(&buf, "<interface type='%s'", type); + + if (managed) + virBufferAddLit(&buf, " managed='yes'>\n"); + else + virBufferAddLit(&buf, ">\n"); virBufferAdjustIndent(&buf, 2); switch (typ) { @@ -995,6 +1006,26 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_DIRECT: virBufferAsprintf(&buf, "<source dev='%s'/>\n", source); break; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + { + struct PCIAddress pciAddr = {0, 0, 0, 0}; + + if (str2PCIAddress(source, &pciAddr) < 0) { + vshError(ctl, _("cannot parse pci address '%s' for network " + "interface"), source); + goto cleanup; + } + + virBufferAddLit(&buf, "<source>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<address type='pci' domain='0x%.4x'" + " bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n", + pciAddr.domain, pciAddr.bus, + pciAddr.slot, pciAddr.function); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</source>\n"); + break; + } case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -1004,7 +1035,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_UDP: case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_LAST: vshError(ctl, _("No support for %s in command 'attach-interface'"), type); -- 2.6.2
On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Adding this feature will allow users to easily attach a hostdev network interface using PCI passthrough.
The interface can be attached using --type=hostdev and PCI address or as --source. This command also allows you to tell, whether the interface should be managed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=997561
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh-domain.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12e85e3..bd00785 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -56,6 +56,7 @@ #include "virtime.h" #include "virtypedparam.h" #include "virxml.h" +#include "virsh-nodedev.h"
/* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -866,6 +867,10 @@ static const vshCmdOptDef opts_attach_interface[] = { .type = VSH_OT_BOOL, .help = N_("print XML document rather than attach the interface") }, + {.name = "managed", + .type = VSH_OT_BOOL, + .help = N_("libvirt will automatically detach/attach the device from/to host") + }, {.name = NULL} };
@@ -931,6 +936,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool persistent = vshCommandOptBool(cmd, "persistent"); + bool managed = vshCommandOptBool(cmd, "managed");
VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
@@ -983,7 +989,12 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) }
/* Make XML of interface */ - virBufferAsprintf(&buf, "<interface type='%s'>\n", type); + virBufferAsprintf(&buf, "<interface type='%s'", type); + + if (managed) + virBufferAddLit(&buf, " managed='yes'>\n"); + else + virBufferAddLit(&buf, ">\n"); virBufferAdjustIndent(&buf, 2);
switch (typ) { @@ -995,6 +1006,26 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_DIRECT: virBufferAsprintf(&buf, "<source dev='%s'/>\n", source); break; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + { + struct PCIAddress pciAddr = {0, 0, 0, 0}; + + if (str2PCIAddress(source, &pciAddr) < 0) { + vshError(ctl, _("cannot parse pci address '%s' for network " + "interface"), source); + goto cleanup; + } + + virBufferAddLit(&buf, "<source>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<address type='pci' domain='0x%.4x'" + " bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n", + pciAddr.domain, pciAddr.bus, + pciAddr.slot, pciAddr.function); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</source>\n"); + break; + }
case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -1004,7 +1035,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_UDP: case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_LAST: vshError(ctl, _("No support for %s in command 'attach-interface'"), type);
What happens if someone provides --managed with some other 'typ'? IOW: If it's only being supported by <hostdev>, then after the switch, a check should probably occur for "if (managed && typ != VIR_DOMAIN_NET_TYPE_HOSTDEV), message, and fail. I'm not fully clear after reading the bz and the previous review whether this patch resolves the bz - something to be worked out in the bz for history's sake though I think with the adjustment to check whether managed is supplied for non hostdev, then you have an ACK for what's changed here. John
On Tue, Oct 27, 2015 at 06:16:33PM -0400, John Ferlan wrote:
On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Adding this feature will allow users to easily attach a hostdev network interface using PCI passthrough.
The interface can be attached using --type=hostdev and PCI address or as --source. This command also allows you to tell, whether the interface should be managed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=997561
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh-domain.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12e85e3..bd00785 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -56,6 +56,7 @@ #include "virtime.h" #include "virtypedparam.h" #include "virxml.h" +#include "virsh-nodedev.h"
/* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -866,6 +867,10 @@ static const vshCmdOptDef opts_attach_interface[] = { .type = VSH_OT_BOOL, .help = N_("print XML document rather than attach the interface") }, + {.name = "managed", + .type = VSH_OT_BOOL, + .help = N_("libvirt will automatically detach/attach the device from/to host") + }, {.name = NULL} };
@@ -931,6 +936,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool persistent = vshCommandOptBool(cmd, "persistent"); + bool managed = vshCommandOptBool(cmd, "managed");
VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
@@ -983,7 +989,12 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) }
/* Make XML of interface */ - virBufferAsprintf(&buf, "<interface type='%s'>\n", type); + virBufferAsprintf(&buf, "<interface type='%s'", type); + + if (managed) + virBufferAddLit(&buf, " managed='yes'>\n"); + else + virBufferAddLit(&buf, ">\n"); virBufferAdjustIndent(&buf, 2);
switch (typ) { @@ -995,6 +1006,26 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_DIRECT: virBufferAsprintf(&buf, "<source dev='%s'/>\n", source); break; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + { + struct PCIAddress pciAddr = {0, 0, 0, 0}; + + if (str2PCIAddress(source, &pciAddr) < 0) { + vshError(ctl, _("cannot parse pci address '%s' for network " + "interface"), source); + goto cleanup; + } + + virBufferAddLit(&buf, "<source>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<address type='pci' domain='0x%.4x'" + " bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n", + pciAddr.domain, pciAddr.bus, + pciAddr.slot, pciAddr.function); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</source>\n"); + break; + }
case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -1004,7 +1035,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_UDP: case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_LAST: vshError(ctl, _("No support for %s in command 'attach-interface'"), type);
What happens if someone provides --managed with some other 'typ'?
IOW: If it's only being supported by <hostdev>, then after the switch, a check should probably occur for "if (managed && typ != VIR_DOMAIN_NET_TYPE_HOSTDEV), message, and fail.
The check was there, but then I removed it because other arguments doesn't check the usability too. We don't need to error out, because libvirt just ignore the "managed='yes'" in the XML.
I'm not fully clear after reading the bz and the previous review whether this patch resolves the bz - something to be worked out in the bz for history's sake though
I think, that the main issue with the BZ is that there was no easy way how to attach *hostdev* interface without manually creating the XML for that interface. We established with Laine, that there is not need for translating netdev name to PCI address.
I think with the adjustment to check whether managed is supplied for non hostdev, then you have an ACK for what's changed here.
Reconsider the 'managed' check, we can be strict and check whether it's used only with hosted type or not, but it's not necessary. Thanks, Pavel
John
On 10/29/2015 03:08 AM, Pavel Hrdina wrote:
On Tue, Oct 27, 2015 at 06:16:33PM -0400, John Ferlan wrote:
[...]
What happens if someone provides --managed with some other 'typ'?
IOW: If it's only being supported by <hostdev>, then after the switch, a check should probably occur for "if (managed && typ != VIR_DOMAIN_NET_TYPE_HOSTDEV), message, and fail.
The check was there, but then I removed it because other arguments doesn't check the usability too. We don't need to error out, because libvirt just ignore the "managed='yes'" in the XML.
I'm not fully clear after reading the bz and the previous review whether this patch resolves the bz - something to be worked out in the bz for history's sake though
I think, that the main issue with the BZ is that there was no easy way how to attach *hostdev* interface without manually creating the XML for that interface. We established with Laine, that there is not need for translating netdev name to PCI address.
I think with the adjustment to check whether managed is supplied for non hostdev, then you have an ACK for what's changed here.
Reconsider the 'managed' check, we can be strict and check whether it's used only with hosted type or not, but it's not necessary.
As I read the docs/code, I see managed is only parsed for <hostdev> types, so yes from that aspect you're correct. I usually err on the side of the extra check so that if some day the parsing code gets changed you don't run into issues. The formatting code certainly only writes out managed='yes' if type is hostdev, so we're safe from the issue of managed='yes' being written into the guest XML... I guess it's the longer way of say ACK for what's here unless you want to be extra paranoid. John
On Thu, Oct 29, 2015 at 10:33:51AM -0400, John Ferlan wrote:
On 10/29/2015 03:08 AM, Pavel Hrdina wrote:
On Tue, Oct 27, 2015 at 06:16:33PM -0400, John Ferlan wrote:
[...]
What happens if someone provides --managed with some other 'typ'?
IOW: If it's only being supported by <hostdev>, then after the switch, a check should probably occur for "if (managed && typ != VIR_DOMAIN_NET_TYPE_HOSTDEV), message, and fail.
The check was there, but then I removed it because other arguments doesn't check the usability too. We don't need to error out, because libvirt just ignore the "managed='yes'" in the XML.
I'm not fully clear after reading the bz and the previous review whether this patch resolves the bz - something to be worked out in the bz for history's sake though
I think, that the main issue with the BZ is that there was no easy way how to attach *hostdev* interface without manually creating the XML for that interface. We established with Laine, that there is not need for translating netdev name to PCI address.
I think with the adjustment to check whether managed is supplied for non hostdev, then you have an ACK for what's changed here.
Reconsider the 'managed' check, we can be strict and check whether it's used only with hosted type or not, but it's not necessary.
As I read the docs/code, I see managed is only parsed for <hostdev> types, so yes from that aspect you're correct. I usually err on the side of the extra check so that if some day the parsing code gets changed you don't run into issues. The formatting code certainly only writes out managed='yes' if type is hostdev, so we're safe from the issue of managed='yes' being written into the guest XML... I guess it's the longer way of say ACK for what's here unless you want to be extra paranoid.
Thanks, I'll push it after freeze. Pavel
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Rewrite the attach-interface section in man page to be more readable and add the new hostdev functionality. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh.pod | 82 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 0212e7a..843c293 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>. [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] -[I<--print-xml>] - -Attach a new network interface to the domain. I<type> can be -I<network> to indicate connection via a libvirt virtual network, or -I<bridge> to indicate connection via a bridge device on the host, or -I<direct> to indicate connection directly to one of the host's network -interfaces or bridges. I<source> indicates the source of the -connection (the name of a network, or of a bridge device, or the -host's network interfaces or bridges). I<target> is used to specify -the tap/macvtap device to be used to connect the domain to the -source. Names starting with 'vnet' are considered as auto-generated -and are blanked out/regenerated each time the interface is attached. -I<mac> specifies the MAC address of the network interface; if a MAC +[I<--managed>] [I<--print-xml>] + +Attach a new network interface to the domain. + +B<--type> can be one of the: I<network> to indicate connection via a libvirt +virtual network, I<bridge> to indicate connection via a bridge device +on the host, I<direct> to indicate connection directly to one of the host's +network interfaces or bridges, I<hostdev> to indicate connection using a +passthrough of PCI device on the host. + +B<--source> indicates the source of the connection. The source depends +on the type of the interface: I<network> name of the virtual network, +I<bridge> the name of the bridge device, I<direct> the name of the host's +interface or bridge, I<hostdev> the PCI address of the host's interface +formatted as domain:bus:slot.function. + +B<--target> is used to specify the tap/macvtap device to be used to +connect the domain to the source. Names starting with 'vnet' are +considered as auto-generated and are blanked out/regenerated each +time the interface is attached. + +B<--mac> specifies the MAC address of the network interface; if a MAC address is not given, a new address will be automatically generated (and stored in the persistent configuration if "--config" is given on -the commandline). I<script> is used to specify a path to a custom -script to be called while attaching to a bridge - this will be called -instead of the default script not in addition to it; --script is valid -only for interfaces of type I<bridge> and only for Xen domains. -I<model> specifies the network device model to be presented to the -domain. I<inbound> and I<outbound> control the bandwidth of the -interface. At least one from the I<average>, I<floor> pair must be -specified. The other two I<peak> and I<burst> are optional, so +the command line). + +B<--script> is used to specify a path to a custom script to be called +while attaching to a bridge - this will be called instead of the default +script not in addition to it; --script is valid only for interfaces of +type I<bridge> and only for Xen domains. + +B<--model> specifies the network device model to be presented to the +domain. + +B<--inbound> and B<--outbound> control the bandwidth of the +interface. At least one from the I<average>, I<floor> pair must be +specified. The other two I<peak> and I<burst> are optional, so "average,peak", "average,,burst", "average,,,floor", "average" and -",,,floor" are also legal. Values for I<average>, I<floor> and I<peak> +",,,floor" are also legal. Values for I<average>, I<floor> and I<peak> are expressed in kilobytes per second, while I<burst> is expressed in kilobytes in a single burst at I<peak> speed as described in the Network XML documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>. -If I<--print-xml> is specified, then the XML of the interface that would be +B<--managed> is usable only for I<hostdev> type and tells libvirt +that the interface should be managed, which means detached and reattached +from/to the host by libvirt. + +If B<--print-xml> is specified, then the XML of the interface that would be attached is printed instead. -If I<--live> is specified, affect a running domain. -If I<--config> is specified, affect the next startup of a persistent domain. -If I<--current> is specified, affect the current domain state. -Both I<--live> and I<--config> flags may be given, but I<--current> is -exclusive. When no flag is specified legacy API is used whose behavior depends -on the hypervisor driver. +If B<--live> is specified, affect a running domain. +If B<--config> is specified, affect the next startup of a persistent domain. +If B<--current> is specified, affect the current domain state. +Both B<--live> and B<--config> flags may be given, but B<--current> is +exclusive. When no flag is specified legacy API is used whose behavior +depends on the hypervisor driver. -For compatibility purposes, I<--persistent> behaves like I<--config> for -an offline domain, and like I<--live> I<--config> for a running domain. +For compatibility purposes, B<--persistent> behaves like B<--config> for +an offline domain, and like B<--live> B<--config> for a running domain. B<Note>: the optional target value is the name of a device to be created -as the back-end on the node. If not provided a device named "vnetN" or "vifN" +as the back-end on the node. If not provided a device named "vnetN" or "vifN" will be created automatically. =item B<detach-device> I<domain> I<FILE> -- 2.6.2
On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Rewrite the attach-interface section in man page to be more readable and add the new hostdev functionality.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh.pod | 82 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 32 deletions(-)
Why a separate patch? It's related to the previous one and if anyone ever (ahem) backed ported the other one, they could miss this one... No strong feeling either way - just curious.
diff --git a/tools/virsh.pod b/tools/virsh.pod index 0212e7a..843c293 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>. [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] -[I<--print-xml>] - -Attach a new network interface to the domain. I<type> can be -I<network> to indicate connection via a libvirt virtual network, or -I<bridge> to indicate connection via a bridge device on the host, or -I<direct> to indicate connection directly to one of the host's network -interfaces or bridges. I<source> indicates the source of the -connection (the name of a network, or of a bridge device, or the -host's network interfaces or bridges). I<target> is used to specify -the tap/macvtap device to be used to connect the domain to the -source. Names starting with 'vnet' are considered as auto-generated -and are blanked out/regenerated each time the interface is attached. -I<mac> specifies the MAC address of the network interface; if a MAC +[I<--managed>] [I<--print-xml>] + +Attach a new network interface to the domain. + +B<--type> can be one of the: I<network> to indicate connection via a libvirt +virtual network, I<bridge> to indicate connection via a bridge device +on the host, I<direct> to indicate connection directly to one of the host's +network interfaces or bridges, I<hostdev> to indicate connection using a +passthrough of PCI device on the host.
Using a ':' I think is unnecessary unless you somehow generate a real list such as via "=item * xxxx" entries. In that case you'd have the following: +B<--type> can be one of the following: + +=over 4 + +=item * Use I<network> to indicate connection via a libvirt virtual network + +=item * Use I<bridge> to indicate connection via a bridge device on the host + + +=item * Use I<direct> to indicate connection directly to one of the host's +network interfaces or bridges + +=item * Use I<hostdev> to indicate connection using a passthrough of PCI device +on the host. + +=back NB: Whether the '--' is required on the type is perhaps a matter of opinion... Since the command line shown doesn't list --type shouldn't this still be an 'B<type>'?
+ +B<--source> indicates the source of the connection. The source depends +on the type of the interface: I<network> name of the virtual network, +I<bridge> the name of the bridge device, I<direct> the name of the host's +interface or bridge, I<hostdev> the PCI address of the host's interface +formatted as domain:bus:slot.function.
Similar comment/construct here and same comment about '--' for source.
+ +B<--target> is used to specify the tap/macvtap device to be used to +connect the domain to the source. Names starting with 'vnet' are +considered as auto-generated and are blanked out/regenerated each +time the interface is attached. + +B<--mac> specifies the MAC address of the network interface; if a MAC
B<--target> and B<--mac> seem OK...
address is not given, a new address will be automatically generated (and stored in the persistent configuration if "--config" is given on -the commandline). I<script> is used to specify a path to a custom -script to be called while attaching to a bridge - this will be called -instead of the default script not in addition to it; --script is valid -only for interfaces of type I<bridge> and only for Xen domains. -I<model> specifies the network device model to be presented to the -domain. I<inbound> and I<outbound> control the bandwidth of the -interface. At least one from the I<average>, I<floor> pair must be -specified. The other two I<peak> and I<burst> are optional, so +the command line). + +B<--script> is used to specify a path to a custom script to be called +while attaching to a bridge - this will be called instead of the default +script not in addition to it; --script is valid only for interfaces of
Existing, but since you're adjusting - should that be I<--script>? a
+type I<bridge> and only for Xen domains.
similarly perhaps "B<type> I<bridge>
+ +B<--model> specifies the network device model to be presented to the +domain. + +B<--inbound> and B<--outbound> control the bandwidth of the +interface. At least one from the I<average>, I<floor> pair must be +specified. The other two I<peak> and I<burst> are optional, so "average,peak", "average,,burst", "average,,,floor", "average" and -",,,floor" are also legal. Values for I<average>, I<floor> and I<peak> +",,,floor" are also legal. Values for I<average>, I<floor> and I<peak>
Not sure the extra space between '.' and start of next sentance is consistent. You changed it here, but not everywhere. There are those that don't like the extra space and those that do. Just be consistent.
are expressed in kilobytes per second, while I<burst> is expressed in kilobytes in a single burst at I<peak> speed as described in the Network XML documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>.
-If I<--print-xml> is specified, then the XML of the interface that would be +B<--managed> is usable only for I<hostdev> type and tells libvirt
for B<type> I<hostdev> ?? An ACK with some adjustments. John
+that the interface should be managed, which means detached and reattached +from/to the host by libvirt. + +If B<--print-xml> is specified, then the XML of the interface that would be attached is printed instead.
-If I<--live> is specified, affect a running domain. -If I<--config> is specified, affect the next startup of a persistent domain. -If I<--current> is specified, affect the current domain state. -Both I<--live> and I<--config> flags may be given, but I<--current> is -exclusive. When no flag is specified legacy API is used whose behavior depends -on the hypervisor driver. +If B<--live> is specified, affect a running domain. +If B<--config> is specified, affect the next startup of a persistent domain. +If B<--current> is specified, affect the current domain state. +Both B<--live> and B<--config> flags may be given, but B<--current> is +exclusive. When no flag is specified legacy API is used whose behavior +depends on the hypervisor driver.
-For compatibility purposes, I<--persistent> behaves like I<--config> for -an offline domain, and like I<--live> I<--config> for a running domain. +For compatibility purposes, B<--persistent> behaves like B<--config> for +an offline domain, and like B<--live> B<--config> for a running domain.
B<Note>: the optional target value is the name of a device to be created -as the back-end on the node. If not provided a device named "vnetN" or "vifN" +as the back-end on the node. If not provided a device named "vnetN" or "vifN" will be created automatically.
=item B<detach-device> I<domain> I<FILE>
On Tue, Oct 27, 2015 at 06:53:55PM -0400, John Ferlan wrote:
On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Rewrite the attach-interface section in man page to be more readable and add the new hostdev functionality.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh.pod | 82 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 32 deletions(-)
Why a separate patch? It's related to the previous one and if anyone ever (ahem) backed ported the other one, they could miss this one... No strong feeling either way - just curious.
It's a rewrite of the attach-interface part of the man page, so I thought, that would be better to do it in a separate patch. Maybe I can at first do the rewrite without adding anything about the new feature and than have the update of man page together with previous patch.
diff --git a/tools/virsh.pod b/tools/virsh.pod index 0212e7a..843c293 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>. [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] -[I<--print-xml>] - -Attach a new network interface to the domain. I<type> can be -I<network> to indicate connection via a libvirt virtual network, or -I<bridge> to indicate connection via a bridge device on the host, or -I<direct> to indicate connection directly to one of the host's network -interfaces or bridges. I<source> indicates the source of the -connection (the name of a network, or of a bridge device, or the -host's network interfaces or bridges). I<target> is used to specify -the tap/macvtap device to be used to connect the domain to the -source. Names starting with 'vnet' are considered as auto-generated -and are blanked out/regenerated each time the interface is attached. -I<mac> specifies the MAC address of the network interface; if a MAC +[I<--managed>] [I<--print-xml>] + +Attach a new network interface to the domain. + +B<--type> can be one of the: I<network> to indicate connection via a libvirt +virtual network, I<bridge> to indicate connection via a bridge device +on the host, I<direct> to indicate connection directly to one of the host's +network interfaces or bridges, I<hostdev> to indicate connection using a +passthrough of PCI device on the host.
Using a ':' I think is unnecessary unless you somehow generate a real list such as via "=item * xxxx" entries. In that case you'd have the following:
I've considered this format before writing the patch and it used a lot of space. I agree, that it would be better arranged. I'll update it.
+B<--type> can be one of the following: + +=over 4 + +=item * Use I<network> to indicate connection via a libvirt virtual network + +=item * Use I<bridge> to indicate connection via a bridge device on the host + + +=item * Use I<direct> to indicate connection directly to one of the host's +network interfaces or bridges + +=item * Use I<hostdev> to indicate connection using a passthrough of PCI device +on the host. + +=back
NB: Whether the '--' is required on the type is perhaps a matter of opinion... Since the command line shown doesn't list --type shouldn't this still be an 'B<type>'?
Oh, You're right, there is no '--type' in the man page. In that case, it should be also referenced the same way. I've used it probably because you can write something like this "attach-interface --domain test --type hostdev ...".
+ +B<--source> indicates the source of the connection. The source depends +on the type of the interface: I<network> name of the virtual network, +I<bridge> the name of the bridge device, I<direct> the name of the host's +interface or bridge, I<hostdev> the PCI address of the host's interface +formatted as domain:bus:slot.function.
Similar comment/construct here and same comment about '--' for source.
+ +B<--target> is used to specify the tap/macvtap device to be used to +connect the domain to the source. Names starting with 'vnet' are +considered as auto-generated and are blanked out/regenerated each +time the interface is attached. + +B<--mac> specifies the MAC address of the network interface; if a MAC
B<--target> and B<--mac> seem OK...
address is not given, a new address will be automatically generated (and stored in the persistent configuration if "--config" is given on -the commandline). I<script> is used to specify a path to a custom -script to be called while attaching to a bridge - this will be called -instead of the default script not in addition to it; --script is valid -only for interfaces of type I<bridge> and only for Xen domains. -I<model> specifies the network device model to be presented to the -domain. I<inbound> and I<outbound> control the bandwidth of the -interface. At least one from the I<average>, I<floor> pair must be -specified. The other two I<peak> and I<burst> are optional, so +the command line). + +B<--script> is used to specify a path to a custom script to be called +while attaching to a bridge - this will be called instead of the default +script not in addition to it; --script is valid only for interfaces of
Existing, but since you're adjusting - should that be I<--script>? a
Good point, I'll rework it to not use the '--script' again, it's clear also without mentioning it again.
+type I<bridge> and only for Xen domains.
similarly perhaps "B<type> I<bridge>
+ +B<--model> specifies the network device model to be presented to the +domain. + +B<--inbound> and B<--outbound> control the bandwidth of the +interface. At least one from the I<average>, I<floor> pair must be +specified. The other two I<peak> and I<burst> are optional, so "average,peak", "average,,burst", "average,,,floor", "average" and -",,,floor" are also legal. Values for I<average>, I<floor> and I<peak> +",,,floor" are also legal. Values for I<average>, I<floor> and I<peak>
Not sure the extra space between '.' and start of next sentance is consistent. You changed it here, but not everywhere. There are those that don't like the extra space and those that do. Just be consistent.
We are not consistent through the man page or the source code about those two vs one space between sentences. I've tried to follow the two space rule for this whole command and in my opinion, it's more readable and makes a good separation between sentences.
are expressed in kilobytes per second, while I<burst> is expressed in kilobytes in a single burst at I<peak> speed as described in the Network XML documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>.
-If I<--print-xml> is specified, then the XML of the interface that would be +B<--managed> is usable only for I<hostdev> type and tells libvirt
for B<type> I<hostdev> ??
I'll try it and see, how it looks like. Thanks, Pavel
An ACK with some adjustments.
John
+that the interface should be managed, which means detached and reattached +from/to the host by libvirt. + +If B<--print-xml> is specified, then the XML of the interface that would be attached is printed instead.
-If I<--live> is specified, affect a running domain. -If I<--config> is specified, affect the next startup of a persistent domain. -If I<--current> is specified, affect the current domain state. -Both I<--live> and I<--config> flags may be given, but I<--current> is -exclusive. When no flag is specified legacy API is used whose behavior depends -on the hypervisor driver. +If B<--live> is specified, affect a running domain. +If B<--config> is specified, affect the next startup of a persistent domain. +If B<--current> is specified, affect the current domain state. +Both B<--live> and B<--config> flags may be given, but B<--current> is +exclusive. When no flag is specified legacy API is used whose behavior +depends on the hypervisor driver.
-For compatibility purposes, I<--persistent> behaves like I<--config> for -an offline domain, and like I<--live> I<--config> for a running domain. +For compatibility purposes, B<--persistent> behaves like B<--config> for +an offline domain, and like B<--live> B<--config> for a running domain.
B<Note>: the optional target value is the name of a device to be created -as the back-end on the node. If not provided a device named "vnetN" or "vifN" +as the back-end on the node. If not provided a device named "vnetN" or "vifN" will be created automatically.
=item B<detach-device> I<domain> I<FILE>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 10/29/2015 04:42 AM, Pavel Hrdina wrote:
On Tue, Oct 27, 2015 at 06:53:55PM -0400, John Ferlan wrote:
On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Rewrite the attach-interface section in man page to be more readable and add the new hostdev functionality.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh.pod | 82 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 32 deletions(-)
Why a separate patch? It's related to the previous one and if anyone ever (ahem) backed ported the other one, they could miss this one... No strong feeling either way - just curious.
It's a rewrite of the attach-interface part of the man page, so I thought, that would be better to do it in a separate patch. Maybe I can at first do the rewrite without adding anything about the new feature and than have the update of man page together with previous patch.
Sure that works too - although I would think mostly unnecessary, but I know that's the norm to separate rewrite from feature/bug fix.
diff --git a/tools/virsh.pod b/tools/virsh.pod index 0212e7a..843c293 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>. [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] -[I<--print-xml>] - -Attach a new network interface to the domain. I<type> can be -I<network> to indicate connection via a libvirt virtual network, or -I<bridge> to indicate connection via a bridge device on the host, or -I<direct> to indicate connection directly to one of the host's network -interfaces or bridges. I<source> indicates the source of the -connection (the name of a network, or of a bridge device, or the -host's network interfaces or bridges). I<target> is used to specify -the tap/macvtap device to be used to connect the domain to the -source. Names starting with 'vnet' are considered as auto-generated -and are blanked out/regenerated each time the interface is attached. -I<mac> specifies the MAC address of the network interface; if a MAC +[I<--managed>] [I<--print-xml>] + +Attach a new network interface to the domain. + +B<--type> can be one of the: I<network> to indicate connection via a libvirt +virtual network, I<bridge> to indicate connection via a bridge device +on the host, I<direct> to indicate connection directly to one of the host's +network interfaces or bridges, I<hostdev> to indicate connection using a +passthrough of PCI device on the host.
Using a ':' I think is unnecessary unless you somehow generate a real list such as via "=item * xxxx" entries. In that case you'd have the following:
I've considered this format before writing the patch and it used a lot of space. I agree, that it would be better arranged. I'll update it.
I contend virsh.pod is a conglomeration of writing and formatting styles. I like your use of B<> instead of I<>, but that is "different". I think separating switches and better/longer descriptions are better, but that's not always the case. The whole file could use some amount of adjustment for consistency in format/style.
+B<--type> can be one of the following: + +=over 4 + +=item * Use I<network> to indicate connection via a libvirt virtual network + +=item * Use I<bridge> to indicate connection via a bridge device on the host + + +=item * Use I<direct> to indicate connection directly to one of the host's +network interfaces or bridges + +=item * Use I<hostdev> to indicate connection using a passthrough of PCI device +on the host. + +=back
NB: Whether the '--' is required on the type is perhaps a matter of opinion... Since the command line shown doesn't list --type shouldn't this still be an 'B<type>'?
Oh, You're right, there is no '--type' in the man page. In that case, it should be also referenced the same way. I've used it probably because you can write something like this "attach-interface --domain test --type hostdev ...".
See this is one of those damned if you do and damned if you don't. One doesn't have to provide "--type" as long as the order is correct. However, providing --type can allow for a different argument order. I don't believe it's ever really described that ARGs can either be specified in the order for each COMMAND, but if not supplied in order, then the --ARG must be used. It's one of those things you just get used to while using virsh.
+ +B<--source> indicates the source of the connection. The source depends +on the type of the interface: I<network> name of the virtual network, +I<bridge> the name of the bridge device, I<direct> the name of the host's +interface or bridge, I<hostdev> the PCI address of the host's interface +formatted as domain:bus:slot.function.
Similar comment/construct here and same comment about '--' for source.
+ +B<--target> is used to specify the tap/macvtap device to be used to +connect the domain to the source. Names starting with 'vnet' are +considered as auto-generated and are blanked out/regenerated each +time the interface is attached. + +B<--mac> specifies the MAC address of the network interface; if a MAC
B<--target> and B<--mac> seem OK...
address is not given, a new address will be automatically generated (and stored in the persistent configuration if "--config" is given on -the commandline). I<script> is used to specify a path to a custom -script to be called while attaching to a bridge - this will be called -instead of the default script not in addition to it; --script is valid -only for interfaces of type I<bridge> and only for Xen domains. -I<model> specifies the network device model to be presented to the -domain. I<inbound> and I<outbound> control the bandwidth of the -interface. At least one from the I<average>, I<floor> pair must be -specified. The other two I<peak> and I<burst> are optional, so +the command line). + +B<--script> is used to specify a path to a custom script to be called +while attaching to a bridge - this will be called instead of the default +script not in addition to it; --script is valid only for interfaces of
Existing, but since you're adjusting - should that be I<--script>? a
Good point, I'll rework it to not use the '--script' again, it's clear also without mentioning it again.
+type I<bridge> and only for Xen domains.
similarly perhaps "B<type> I<bridge>
+ +B<--model> specifies the network device model to be presented to the +domain. + +B<--inbound> and B<--outbound> control the bandwidth of the +interface. At least one from the I<average>, I<floor> pair must be +specified. The other two I<peak> and I<burst> are optional, so "average,peak", "average,,burst", "average,,,floor", "average" and -",,,floor" are also legal. Values for I<average>, I<floor> and I<peak> +",,,floor" are also legal. Values for I<average>, I<floor> and I<peak>
Not sure the extra space between '.' and start of next sentance is consistent. You changed it here, but not everywhere. There are those that don't like the extra space and those that do. Just be consistent.
We are not consistent through the man page or the source code about those two vs one space between sentences. I've tried to follow the two space rule for this whole command and in my opinion, it's more readable and makes a good separation between sentences.
Yes - being consistent is difficult. I know some like ". Next" while others prefer ". Next". I sometimes find myself going back and removing the extra space, while other times I just don't bother. I think my fingers have been trained through the years of doing this to add two spaces after a period. See I removed them here because I was thinking about it, but before I wasn't so they crept in! John
are expressed in kilobytes per second, while I<burst> is expressed in kilobytes in a single burst at I<peak> speed as described in the Network XML documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>.
-If I<--print-xml> is specified, then the XML of the interface that would be +B<--managed> is usable only for I<hostdev> type and tells libvirt
for B<type> I<hostdev> ??
I'll try it and see, how it looks like.
Thanks,
Pavel
An ACK with some adjustments.
John
+that the interface should be managed, which means detached and reattached +from/to the host by libvirt. + +If B<--print-xml> is specified, then the XML of the interface that would be attached is printed instead.
-If I<--live> is specified, affect a running domain. -If I<--config> is specified, affect the next startup of a persistent domain. -If I<--current> is specified, affect the current domain state. -Both I<--live> and I<--config> flags may be given, but I<--current> is -exclusive. When no flag is specified legacy API is used whose behavior depends -on the hypervisor driver. +If B<--live> is specified, affect a running domain. +If B<--config> is specified, affect the next startup of a persistent domain. +If B<--current> is specified, affect the current domain state. +Both B<--live> and B<--config> flags may be given, but B<--current> is +exclusive. When no flag is specified legacy API is used whose behavior +depends on the hypervisor driver.
-For compatibility purposes, I<--persistent> behaves like I<--config> for -an offline domain, and like I<--live> I<--config> for a running domain. +For compatibility purposes, B<--persistent> behaves like B<--config> for +an offline domain, and like B<--live> B<--config> for a running domain.
B<Note>: the optional target value is the name of a device to be created -as the back-end on the node. If not provided a device named "vnetN" or "vifN" +as the back-end on the node. If not provided a device named "vnetN" or "vifN" will be created automatically.
=item B<detach-device> I<domain> I<FILE>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Oct 29, 2015 at 10:52:00AM -0400, John Ferlan wrote:
On 10/29/2015 04:42 AM, Pavel Hrdina wrote:
On Tue, Oct 27, 2015 at 06:53:55PM -0400, John Ferlan wrote:
On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
Rewrite the attach-interface section in man page to be more readable and add the new hostdev functionality.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tools/virsh.pod | 82 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 32 deletions(-)
Why a separate patch? It's related to the previous one and if anyone ever (ahem) backed ported the other one, they could miss this one... No strong feeling either way - just curious.
It's a rewrite of the attach-interface part of the man page, so I thought, that would be better to do it in a separate patch. Maybe I can at first do the rewrite without adding anything about the new feature and than have the update of man page together with previous patch.
Sure that works too - although I would think mostly unnecessary, but I know that's the norm to separate rewrite from feature/bug fix.
It's not that hard to split the patch, so I'll do it.
diff --git a/tools/virsh.pod b/tools/virsh.pod index 0212e7a..843c293 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>. [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] -[I<--print-xml>] - -Attach a new network interface to the domain. I<type> can be -I<network> to indicate connection via a libvirt virtual network, or -I<bridge> to indicate connection via a bridge device on the host, or -I<direct> to indicate connection directly to one of the host's network -interfaces or bridges. I<source> indicates the source of the -connection (the name of a network, or of a bridge device, or the -host's network interfaces or bridges). I<target> is used to specify -the tap/macvtap device to be used to connect the domain to the -source. Names starting with 'vnet' are considered as auto-generated -and are blanked out/regenerated each time the interface is attached. -I<mac> specifies the MAC address of the network interface; if a MAC +[I<--managed>] [I<--print-xml>] + +Attach a new network interface to the domain. + +B<--type> can be one of the: I<network> to indicate connection via a libvirt +virtual network, I<bridge> to indicate connection via a bridge device +on the host, I<direct> to indicate connection directly to one of the host's +network interfaces or bridges, I<hostdev> to indicate connection using a +passthrough of PCI device on the host.
Using a ':' I think is unnecessary unless you somehow generate a real list such as via "=item * xxxx" entries. In that case you'd have the following:
I've considered this format before writing the patch and it used a lot of space. I agree, that it would be better arranged. I'll update it.
I contend virsh.pod is a conglomeration of writing and formatting styles. I like your use of B<> instead of I<>, but that is "different". I think separating switches and better/longer descriptions are better, but that's not always the case. The whole file could use some amount of adjustment for consistency in format/style.
I agree, the man page is a mess and really hard to read and understand. Pavel
participants (2)
-
John Ferlan -
Pavel Hrdina