2:17 a.m.
From: Lin Ma <lma(a)suse.de>
v1 -> v2:
* Remove the merged patches.
* Follow Ján and Erik's suggestions.
Lin Ma (4):
virsh: net-port-create: log errors for non-existent xml file
virsh: event: options --list, --all and --event are mutually exclusive
docs: virsh: Document the IO mode 'io_uring'
network: Check for active network during networkGetDHCPLeases
docs/manpages/virsh.rst | 3 ++-
src/network/bridge_driver.c | 7 +++++++
tools/virsh-domain.c | 4 ++++
tools/virsh-network.c | 1 +
4 files changed, 14 insertions(+), 1 deletion(-)
--
2.26.0
2:17 a.m.
New subject: [libvirt] [PATCH v2 1/4] virsh: net-port-create: log errors for non-existent xml file
From: Lin Ma <lma(a)suse.de>
Signed-off-by: Lin Ma <lma(a)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 0351a93f19..24bc9107fc 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1523,6 +1523,7 @@ cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
+ vshSaveLibvirtError();
VIR_FREE(buffer);
if (port)
virNetworkPortFree(port);
--
2.26.0
5:40 a.m.
New subject: [libvirt] [PATCH v2 1/4] virsh: net-port-create: log errors for non-existent xml file
On 9/16/20 9:17 AM, morecache(a)gmail.com wrote:
From: Lin Ma <lma(a)suse.de>
Signed-off-by: Lin Ma <lma(a)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 0351a93f19..24bc9107fc 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1523,6 +1523,7 @@ cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
+ vshSaveLibvirtError();
I believe this must go under virFileReadAll():
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
vshSaveLibvirtError();
goto cleanup;
}
We don't want to be reporting libvirt error every time we reach the
label, because not all jumps onto that label are direct result of a
failing function that uses virReportError().
Michal
2:17 a.m.
New subject: [libvirt] [PATCH v2 2/4] virsh: event: options --list, --all and --event are mutually exclusive
From: Lin Ma <lma(a)suse.de>
Signed-off-by: Lin Ma <lma(a)suse.de>
---
tools/virsh-domain.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f0ca6f7b83..32edfc0398 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13710,6 +13710,10 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
int count = 0;
virshControlPtr priv = ctl->privData;
+ VSH_EXCLUSIVE_OPTIONS("all", "event");
+ VSH_EXCLUSIVE_OPTIONS("list", "all");
+ VSH_EXCLUSIVE_OPTIONS("list", "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
2:17 a.m.
New subject: [libvirt] [PATCH v2 3/4] docs: virsh: Document the IO mode 'io_uring'
From: Lin Ma <lma(a)suse.de>
Signed-off-by: Lin Ma <lma(a)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 9fd5efed0f..bf84f95678 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" and "io_uring".
*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
2:17 a.m.
New subject: [libvirt] [PATCH v2 4/4] network: Check for active network during networkGetDHCPLeases
From: Lin Ma <lma(a)suse.de>
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(a)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
5:40 a.m.
New subject: [libvirt] [PATCH v2 4/4] network: Check for active network during networkGetDHCPLeases
On 9/16/20 9:17 AM, morecache(a)gmail.com wrote:
From: Lin Ma <lma(a)suse.de>
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(a)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;
No need to jump all the way to 'error' when 'cleanup' is just fine.
@leases_ret wasn't touched until now and hence is still NULL, this
'error' is the same as 'cleanup'. But you can keep it, if you want,
we're jumping "randomly" on error and cleanup.
Michal
7:18 a.m.
New subject: [libvirt] [PATCH v2 4/4] network: Check for active network during networkGetDHCPLeases
On a Wednesday in 2020, Michal Privoznik wrote:
On 9/16/20 9:17 AM, morecache(a)gmail.com wrote:
>From: Lin Ma <lma(a)suse.de>
>
>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.
>
NACK, see discussion on v1.
https://www.redhat.com/archives/libvir-list/2020-September/msg00673.html
Jano
>Signed-off-by: Lin Ma <lma(a)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;
No need to jump all the way to 'error' when 'cleanup' is just fine.
@leases_ret wasn't touched until now and hence is still NULL, this
'error' is the same as 'cleanup'. But you can keep it, if you want,
we're jumping "randomly" on error and cleanup.
Michal
8:15 a.m.
New subject: [libvirt] [PATCH v2 4/4] network: Check for active network during networkGetDHCPLeases
On 9/16/20 2:18 PM, Ján Tomko wrote:
On a Wednesday in 2020, Michal Privoznik wrote:
> On 9/16/20 9:17 AM, morecache(a)gmail.com wrote:
>> From: Lin Ma <lma(a)suse.de>
>>
>> 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.
>>
NACK, see discussion on v1.
https://www.redhat.com/archives/libvir-list/2020-September/msg00673.html
So what if I set up a static lease in network XM, start the network,
then shut it down and change the XML? Wouldn't I get a different lease
until the network is started again?
The NSS plugin suffers from the same problem, btw. But it can't know
whether the network is still running or not (if we don't want it to do
some hacks).
Michal
5:40 a.m.
On 9/16/20 9:17 AM, morecache(a)gmail.com wrote:
From: Lin Ma <lma(a)suse.de>
v1 -> v2:
* Remove the merged patches.
* Follow Ján and Erik's suggestions.
Lin Ma (4):
virsh: net-port-create: log errors for non-existent xml file
virsh: event: options --list, --all and --event are mutually exclusive
docs: virsh: Document the IO mode 'io_uring'
network: Check for active network during networkGetDHCPLeases
docs/manpages/virsh.rst | 3 ++-
src/network/bridge_driver.c | 7 +++++++
tools/virsh-domain.c | 4 ++++
tools/virsh-network.c | 1 +
4 files changed, 14 insertions(+), 1 deletion(-)
Patches looks good to me, but see my comments. I can do the changes
before pushing, if you agree with suggested changes.
Michal
10:20 p.m.
On 2020-09-16 10:40, Michal Privoznik wrote:
On 9/16/20 9:17 AM, morecache(a)gmail.com wrote:
> From: Lin Ma <lma(a)suse.de>
>
> v1 -> v2:
>
> * Remove the merged patches.
> * Follow Ján and Erik's suggestions.
>
> Lin Ma (4):
> virsh: net-port-create: log errors for non-existent xml file
> virsh: event: options --list, --all and --event are mutually
> exclusive
> docs: virsh: Document the IO mode 'io_uring'
> network: Check for active network during networkGetDHCPLeases
>
> docs/manpages/virsh.rst | 3 ++-
> src/network/bridge_driver.c | 7 +++++++
> tools/virsh-domain.c | 4 ++++
> tools/virsh-network.c | 1 +
> 4 files changed, 14 insertions(+), 1 deletion(-)
>
Patches looks good to me, but see my comments. I can do the changes
before pushing, if you agree with suggested changes.
No problem, Thank you very much.
Lin
5:32 a.m.
On 9/17/20 5:20 AM, Lin Ma wrote:
On 2020-09-16 10:40, Michal Privoznik wrote:
> On 9/16/20 9:17 AM, morecache(a)gmail.com wrote:
>> From: Lin Ma <lma(a)suse.de>
>>
>> v1 -> v2:
>>
>> * Remove the merged patches.
>> * Follow Ján and Erik's suggestions.
>>
>> Lin Ma (4):
>> virsh: net-port-create: log errors for non-existent xml file
>> virsh: event: options --list, --all and --event are mutually
>> exclusive
>> docs: virsh: Document the IO mode 'io_uring'
>> network: Check for active network during networkGetDHCPLeases
>>
>> docs/manpages/virsh.rst | 3 ++-
>> src/network/bridge_driver.c | 7 +++++++
>> tools/virsh-domain.c | 4 ++++
>> tools/virsh-network.c | 1 +
>> 4 files changed, 14 insertions(+), 1 deletion(-)
>>
>
> Patches looks good to me, but see my comments. I can do the changes
> before pushing, if you agree with suggested changes.
No problem, Thank you very much.
Since Jano didn't like 4/4, I'm pushing 1-3/4 only.
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1558
days inactive
1559
days old
11 comments
4 participants
participants (4)
-
Ján Tomko -
Lin Ma -
Michal Privoznik -
morecache＠gmail.com