[libvirt] [PATCH 00/12] qemu/virsh/docs: various minor fixes
Lin Ma (12): qemu: qemuDomainPMSuspendForDuration: Check availability of agent qemu: Return perf status that affect next boot for shutoff domains network: Check for active network during networkGetDHCPLeases virsh: domid: Error out if domain is not active virsh: net-port-create: log errors for non-existent xml file virsh: event: options --all and --event are mutually exclusive virsh: domblkinfo: options --all and --device are mutually exclusive virsh: domdisplay: options --all and --type are mutually exclusive virsh: iface-list: options --all and --inactive are mutually exclusive virsh: pool-list: options --all and --inactive are mutually exclusive docs: virsh: Document the IO mode 'io_uring' docs: virsh: Drop duplicate spelling for dompmwakeup docs/manpages/virsh.rst | 5 +++-- src/network/bridge_driver.c | 7 +++++++ src/qemu/qemu_driver.c | 8 ++++++-- tools/virsh-domain-monitor.c | 2 ++ tools/virsh-domain.c | 17 ++++++++++++----- tools/virsh-interface.c | 2 ++ tools/virsh-network.c | 1 + tools/virsh-pool.c | 2 ++ 8 files changed, 35 insertions(+), 9 deletions(-) -- 2.26.0
It requires a guest agent configured and running in the domain's guest OS, So check qemu agent during qemuDomainPMSuspendForDuration(). Signed-off-by: Lin Ma <lma@suse.de> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e46931c71..bd287f259e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16820,6 +16820,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (!qemuDomainAgentAvailable(vm, true)) + goto cleanup; + /* * The case we want to handle here is when QEMU has the API (i.e. * QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere -- 2.26.0
On Fri, Sep 11, 2020 at 03:06:08PM +0800, Lin Ma wrote:
It requires a guest agent configured and running in the domain's guest OS, So check qemu agent during qemuDomainPMSuspendForDuration().
Signed-off-by: Lin Ma <lma@suse.de> ---
qemuDomainPMSuspendAgent later in that same function already checks it, we don't need to check it twice. NACK
src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e46931c71..bd287f259e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16820,6 +16820,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
+ if (!qemuDomainAgentAvailable(vm, true)) + goto cleanup; + /* * The case we want to handle here is when QEMU has the API (i.e. * QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere -- 2.26.0
On a Friday in 2020, Erik Skultety wrote:
On Fri, Sep 11, 2020 at 03:06:08PM +0800, Lin Ma wrote:
It requires a guest agent configured and running in the domain's guest OS, So check qemu agent during qemuDomainPMSuspendForDuration().
Signed-off-by: Lin Ma <lma@suse.de> ---
qemuDomainPMSuspendAgent later in that same function already checks it, we don't need to check it twice.
It only does so after calling qemuDomainQueryWakeupSuspendSupport, which requires grabbing a MODIFY job and executing a QMP command. I think doing this check upfront is reasonable. We will error out as soon as we know it, just for the price for two extra duplicit lines. Jano
NACK
src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e46931c71..bd287f259e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16820,6 +16820,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
+ if (!qemuDomainAgentAvailable(vm, true)) + goto cleanup; + /* * The case we want to handle here is when QEMU has the API (i.e. * QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere -- 2.26.0
On Fri, Sep 11, 2020 at 01:39:32PM +0200, Ján Tomko wrote:
On a Friday in 2020, Erik Skultety wrote:
On Fri, Sep 11, 2020 at 03:06:08PM +0800, Lin Ma wrote:
It requires a guest agent configured and running in the domain's guest OS, So check qemu agent during qemuDomainPMSuspendForDuration().
Signed-off-by: Lin Ma <lma@suse.de> ---
qemuDomainPMSuspendAgent later in that same function already checks it, we don't need to check it twice.
It only does so after calling qemuDomainQueryWakeupSuspendSupport, which requires grabbing a MODIFY job and executing a QMP command.
Why does a query grab a MODIFY job anyway?
I think doing this check upfront is reasonable. We will error out as soon as we know it, just for the price for two extra duplicit lines.
True, it will save a fraction of time, I guess I could live with that. More importantly though, this patch uncovered a bug in the code where we report a successful suspend even when the agent is not connected and we logged an error accordingly. Erik
Jano
NACK
src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e46931c71..bd287f259e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16820,6 +16820,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
+ if (!qemuDomainAgentAvailable(vm, true)) + goto cleanup; + /* * The case we want to handle here is when QEMU has the API (i.e. * QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
It requires a guest agent configured and running in the domain's guest OS, So check qemu agent during qemuDomainPMSuspendForDuration().
Signed-off-by: Lin Ma <lma@suse.de> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
While we set up perf events for a shutoff domain and check the settings, All of perf events are reported as 'disabled', unless we add --config, This is redundant for a shutoff domain. # virsh domstate $GUEST shut off # virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ...... # virsh perf --domain $GUEST --enable mbmt mbmt : enabled # virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ...... Use virDomainObjGetOneDefState instead of virDomainObjGetOneDef to fix the issue. After patch, The perf event status of a shutoff domain is reported correctly: # virsh domstate $GUEST shut off # virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ...... # virsh perf --domain $GUEST --enable mbmt mbmt : enabled # virsh perf --domain $GUEST cmt : disabled mbmt : enabled mbml : disabled ...... Signed-off-by: Lin Ma <lma@suse.de> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bd287f259e..717afcd34d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9109,6 +9109,7 @@ qemuDomainGetPerfEvents(virDomainPtr dom, int npar = 0; size_t i; int ret = -1; + bool live = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -9123,7 +9124,7 @@ qemuDomainGetPerfEvents(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; - if (!(def = virDomainObjGetOneDef(vm, flags))) + if (!(def = virDomainObjGetOneDefState(vm, flags, &live))) goto endjob; priv = vm->privateData; @@ -9131,7 +9132,7 @@ qemuDomainGetPerfEvents(virDomainPtr dom, for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { bool perf_enabled; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) || !live) perf_enabled = def->perf.events[i] == VIR_TRISTATE_BOOL_YES; else perf_enabled = virPerfEventIsEnabled(priv->perf, i); -- 2.26.0
On Fri, Sep 11, 2020 at 03:06:09PM +0800, Lin Ma wrote:
While we set up perf events for a shutoff domain and check the settings, All of perf events are reported as 'disabled', unless we add --config, This is redundant for a shutoff domain.
# virsh domstate $GUEST shut off
# virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ......
# virsh perf --domain $GUEST --enable mbmt mbmt : enabled
# virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ......
Use virDomainObjGetOneDefState instead of virDomainObjGetOneDef to fix the issue. After patch, The perf event status of a shutoff domain is reported correctly:
# virsh domstate $GUEST shut off
# virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ......
# virsh perf --domain $GUEST --enable mbmt mbmt : enabled
# virsh perf --domain $GUEST cmt : disabled mbmt : enabled mbml : disabled ......
Signed-off-by: Lin Ma <lma@suse.de> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
On a Friday in 2020, Lin Ma wrote:
While we set up perf events for a shutoff domain and check the settings, All of perf events are reported as 'disabled', unless we add --config, This is redundant for a shutoff domain.
# virsh domstate $GUEST shut off
# virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ......
# virsh perf --domain $GUEST --enable mbmt mbmt : enabled
# virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ......
Use virDomainObjGetOneDefState instead of virDomainObjGetOneDef to fix the issue. After patch, The perf event status of a shutoff domain is reported correctly:
# virsh domstate $GUEST shut off
# virsh perf --domain $GUEST cmt : disabled mbmt : disabled mbml : disabled ......
# virsh perf --domain $GUEST --enable mbmt mbmt : enabled
# virsh perf --domain $GUEST cmt : disabled mbmt : enabled mbml : disabled ......
Signed-off-by: Lin Ma <lma@suse.de> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
It doesn't make sense querying dhcp leases for interfaces against an inactive network, This patch adds a check to see if the network is active. Signed-off-by: Lin Ma <lma@suse.de> --- src/network/bridge_driver.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 87d7acab06..1dffc2309f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net, if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0) goto cleanup; + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto error; + } + /* Retrieve custom leases file location */ custom_lease_file = networkDnsmasqLeaseFileNameCustom(driver, def->bridge); -- 2.26.0
On Fri, Sep 11, 2020 at 03:06:10PM +0800, Lin Ma wrote:
It doesn't make sense querying dhcp leases for interfaces against an inactive network, This patch adds a check to see if the network is active.
Signed-off-by: Lin Ma <lma@suse.de> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
On a Friday in 2020, Lin Ma wrote:
It doesn't make sense querying dhcp leases for interfaces against an inactive network, This patch adds a check to see if the network is active.
Why would the network need to be active? the leases are still there: $ virsh net-destroy default Network default destroyed $ virsh net-dhcp-leases default Expiry Time MAC address Protocol IP address Hostname Client ID or DUID ------------------------------------------------------------------------------------------------------------ 2020-09-11 16:16:59 52:54:00:55:7c:df ipv4 192.168.122.183/24 - 01:52:54:00:55:7c:df Jano
Signed-off-by: Lin Ma <lma@suse.de> --- src/network/bridge_driver.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 87d7acab06..1dffc2309f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net, if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0) goto cleanup;
+ if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto error; + } + /* Retrieve custom leases file location */ custom_lease_file = networkDnsmasqLeaseFileNameCustom(driver, def->bridge);
-- 2.26.0
On 2020-09-11 13:22, Ján Tomko wrote:
On a Friday in 2020, Lin Ma wrote:
It doesn't make sense querying dhcp leases for interfaces against an inactive network, This patch adds a check to see if the network is active.
Why would the network need to be active? the leases are still there:
$ virsh net-destroy default Network default destroyed
$ virsh net-dhcp-leases default Expiry Time MAC address Protocol IP address Hostname Client ID or DUID ------------------------------------------------------------------------------------------------------------ 2020-09-11 16:16:59 52:54:00:55:7c:df ipv4 192.168.122.183/24 - 01:52:54:00:55:7c:df
IMO the conception of leases doesn't make sense in case of no dhcp server daemon running. I'll carry this patch in V2 patch set, Please feel free to decline it. Thanks, Lin
--- src/network/bridge_driver.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 87d7acab06..1dffc2309f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net, if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0) goto cleanup;
+ if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto error; + } + /* Retrieve custom leases file location */ custom_lease_file = networkDnsmasqLeaseFileNameCustom(driver, def->bridge);
-- 2.26.0
On a Wednesday in 2020, Lin Ma wrote:
On 2020-09-11 13:22, Ján Tomko wrote:
On a Friday in 2020, Lin Ma wrote:
It doesn't make sense querying dhcp leases for interfaces against an inactive network, This patch adds a check to see if the network is active.
Why would the network need to be active? the leases are still there:
$ virsh net-destroy default Network default destroyed
$ virsh net-dhcp-leases default Expiry Time MAC address Protocol IP address Hostname Client ID or DUID ------------------------------------------------------------------------------------------------------------ 2020-09-11 16:16:59 52:54:00:55:7c:df ipv4 192.168.122.183/24 - 01:52:54:00:55:7c:df
IMO the conception of leases doesn't make sense in case of no dhcp server daemon running.
The leases are still there and will be picked up by the DHCP server after it's started again. We do have access to the leases even if the server is not running. Why would we deny querying it? Jano
I'll carry this patch in V2 patch set, Please feel free to decline it.
Thanks, Lin
--- src/network/bridge_driver.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 87d7acab06..1dffc2309f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net, if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0) goto cleanup;
+ if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto error; + } + /* Retrieve custom leases file location */ custom_lease_file = networkDnsmasqLeaseFileNameCustom(driver, def->bridge);
-- 2.26.0
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d1d3f8e566..86152a53b1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10458,18 +10458,21 @@ cmdDomid(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; unsigned int id; + bool ret = false; if (!(dom = virshCommandOptDomainBy(ctl, cmd, NULL, VIRSH_BYNAME|VIRSH_BYUUID))) - return false; + return ret; id = virDomainGetID(dom); - if (id == ((unsigned int)-1)) - vshPrint(ctl, "%s\n", "-"); - else + if (id == ((unsigned int)-1)) { + vshError(ctl, "%s", _("Domain is not active")); + } else { vshPrint(ctl, "%d\n", id); + ret = true; + } virshDomainFree(dom); - return true; + return ret; } /* -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d1d3f8e566..86152a53b1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10458,18 +10458,21 @@ cmdDomid(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; unsigned int id; + bool ret = false;
if (!(dom = virshCommandOptDomainBy(ctl, cmd, NULL, VIRSH_BYNAME|VIRSH_BYUUID))) - return false; + return ret;
id = virDomainGetID(dom); - if (id == ((unsigned int)-1)) - vshPrint(ctl, "%s\n", "-"); - else + if (id == ((unsigned int)-1)) { + vshError(ctl, "%s", _("Domain is not active"));
I'm not convinced we should change the existing behavior here. Jano
+ } else { vshPrint(ctl, "%d\n", id); + ret = true; + } virshDomainFree(dom); - return true; + return ret; }
/* -- 2.26.0
On 2020-09-12 10:31, Ján Tomko wrote:
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d1d3f8e566..86152a53b1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10458,18 +10458,21 @@ cmdDomid(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; unsigned int id; + bool ret = false;
if (!(dom = virshCommandOptDomainBy(ctl, cmd, NULL, VIRSH_BYNAME|VIRSH_BYUUID))) - return false; + return ret;
id = virDomainGetID(dom); - if (id == ((unsigned int)-1)) - vshPrint(ctl, "%s\n", "-"); - else + if (id == ((unsigned int)-1)) { + vshError(ctl, "%s", _("Domain is not active"));
I'm not convinced we should change the existing behavior here.
Jano
ok, let's leave it as it is. Thanks, Lin
+ } else { vshPrint(ctl, "%d\n", id); + ret = true; + } virshDomainFree(dom); - return true; + return ret; }
/* -- 2.26.0
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-network.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index f0f5358625..ce67e3f19b 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1522,6 +1522,7 @@ cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: + vshReportError(ctl); VIR_FREE(buffer); if (port) virNetworkPortFree(port); -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-network.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c index f0f5358625..ce67e3f19b 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1522,6 +1522,7 @@ cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd)
ret = true; cleanup: + vshReportError(ctl);
vshReportError is already called in vshCommandRun, all that's needed here is a vshSaveLibvirtError call, before the error gets reset by virNetwork*Free APIs Jano
VIR_FREE(buffer); if (port) virNetworkPortFree(port); -- 2.26.0
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 86152a53b1..657a0532db 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13696,6 +13696,8 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd) int count = 0; virshControlPtr priv = ctl->privData; + VSH_EXCLUSIVE_OPTIONS("all", "event"); + if (vshCommandOptBool(cmd, "list")) { for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++) vshPrint(ctl, "%s\n", virshDomainEventCallbacks[event].name); -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 86152a53b1..657a0532db 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13696,6 +13696,8 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd) int count = 0; virshControlPtr priv = ctl->privData;
+ VSH_EXCLUSIVE_OPTIONS("all", "event");
--list also excludes any of these two. Jano
+ if (vshCommandOptBool(cmd, "list")) { for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++) vshPrint(ctl, "%s\n", virshDomainEventCallbacks[event].name); -- 2.26.0
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain-monitor.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 0111116885..a813476936 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -469,6 +469,8 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) char *phy = NULL; vshTablePtr table = NULL; + VSH_EXCLUSIVE_OPTIONS("all", "device"); + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain-monitor.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 657a0532db..8d7f45091c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11526,6 +11526,8 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/%s)"; virSocketAddr addr; + VSH_EXCLUSIVE_OPTIONS("all", "type"); + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-domain.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-interface.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index a6b52726c9..2bca2483dd 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -351,6 +351,8 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) bool ret = false; vshTablePtr table = NULL; + VSH_EXCLUSIVE_OPTIONS_VAR(all, inactive); + if (inactive) flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE; if (all) -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-interface.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-pool.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 9fac590ec3..634e9ac8cd 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1146,6 +1146,8 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) inactive = vshCommandOptBool(cmd, "inactive"); all = vshCommandOptBool(cmd, "all"); + VSH_EXCLUSIVE_OPTIONS_VAR(all, inactive); + if (inactive) flags = VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE; -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- tools/virsh-pool.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Lin Ma <lma@suse.de> --- docs/manpages/virsh.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index ca5acf84ca..b8206205f6 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4521,7 +4521,8 @@ within the existing virtual cdrom or floppy device; consider using *sourcetype* can indicate the type of source (block|file) *cache* can be one of "default", "none", "writethrough", "writeback", "directsync" or "unsafe". -*io* controls specific policies on I/O; QEMU guests support "threads" and "native". +*io* controls specific policies on I/O; QEMU guests support "threads", +"native", "io_uring" since kernel 5.1 and qemu 5.0. *iothread* is the number within the range of domain IOThreads to which this disk may be attached (QEMU only). *serial* is the serial of disk device. *wwn* is the wwn of disk device. -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- docs/manpages/virsh.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index ca5acf84ca..b8206205f6 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4521,7 +4521,8 @@ within the existing virtual cdrom or floppy device; consider using *sourcetype* can indicate the type of source (block|file) *cache* can be one of "default", "none", "writethrough", "writeback", "directsync" or "unsafe". -*io* controls specific policies on I/O; QEMU guests support "threads" and "native". +*io* controls specific policies on I/O; QEMU guests support "threads", +"native", "io_uring" since kernel 5.1 and qemu 5.0.
Threads and native were supported even with older QEMUs and kernels. The phrasing here looks ambiguous. Jano
*iothread* is the number within the range of domain IOThreads to which this disk may be attached (QEMU only). *serial* is the serial of disk device. *wwn* is the wwn of disk device. -- 2.26.0
On Sat, Sep 12, 2020 at 12:48:13PM +0200, Ján Tomko wrote:
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- docs/manpages/virsh.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index ca5acf84ca..b8206205f6 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4521,7 +4521,8 @@ within the existing virtual cdrom or floppy device; consider using *sourcetype* can indicate the type of source (block|file) *cache* can be one of "default", "none", "writethrough", "writeback", "directsync" or "unsafe". -*io* controls specific policies on I/O; QEMU guests support "threads" and "native". +*io* controls specific policies on I/O; QEMU guests support "threads", +"native", "io_uring" since kernel 5.1 and qemu 5.0.
I don't see why we should mention the specific versions in the manpage for the client, it should be documented on the backend side IOW libvirt docs. The client just gets support for a new feature, everything else doesn't need to concern it IMO, so I'd just go simply with "io_uring" and by that you also drop the ambiguity Jano mentioned. Erik
Threads and native were supported even with older QEMUs and kernels. The phrasing here looks ambiguous.
Jano
*iothread* is the number within the range of domain IOThreads to which this disk may be attached (QEMU only). *serial* is the serial of disk device. *wwn* is the wwn of disk device. -- 2.26.0
Signed-off-by: Lin Ma <lma@suse.de> --- docs/manpages/virsh.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index b8206205f6..963a27c031 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2156,7 +2156,7 @@ dompmwakeup Wakeup a domain from pmsuspended state (either suspended by dompmsuspend or from the guest itself). Injects a wakeup into the guest that is in pmsuspended state, rather than waiting for the previously requested duration (if any) to -elapse. This operation doesn't not necessarily fail if the domain is running. +elapse. This operation does not necessarily fail if the domain is running. domrename -- 2.26.0
On a Friday in 2020, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.de> --- docs/manpages/virsh.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On a Friday in 2020, Lin Ma wrote:
Lin Ma (12): qemu: qemuDomainPMSuspendForDuration: Check availability of agent virsh: domblkinfo: options --all and --device are mutually exclusive virsh: domdisplay: options --all and --type are mutually exclusive virsh: iface-list: options --all and --inactive are mutually exclusive virsh: pool-list: options --all and --inactive are mutually exclusive docs: virsh: Drop duplicate spelling for dompmwakeup
I've pushed the patches above ^^^ Jano
qemu: Return perf status that affect next boot for shutoff domains network: Check for active network during networkGetDHCPLeases virsh: domid: Error out if domain is not active virsh: net-port-create: log errors for non-existent xml file virsh: event: options --all and --event are mutually exclusive docs: virsh: Document the IO mode 'io_uring'
docs/manpages/virsh.rst | 5 +++-- src/network/bridge_driver.c | 7 +++++++ src/qemu/qemu_driver.c | 8 ++++++-- tools/virsh-domain-monitor.c | 2 ++ tools/virsh-domain.c | 17 ++++++++++++----- tools/virsh-interface.c | 2 ++ tools/virsh-network.c | 1 + tools/virsh-pool.c | 2 ++ 8 files changed, 35 insertions(+), 9 deletions(-)
participants (4)
-
Erik Skultety -
Ján Tomko -
Lin Ma -
Lin Ma