[libvirt PATCH 1/2] tests: Tweak input file

2023-04-11 Thread Andrea Bolognani
The canonical order for  child elements is 
then .

Signed-off-by: Andrea Bolognani 
---
 tests/qemuxml2argvdata/firmware-manual-efi-features.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemuxml2argvdata/firmware-manual-efi-features.xml 
b/tests/qemuxml2argvdata/firmware-manual-efi-features.xml
index 8e5aff7559..092739af56 100644
--- a/tests/qemuxml2argvdata/firmware-manual-efi-features.xml
+++ b/tests/qemuxml2argvdata/firmware-manual-efi-features.xml
@@ -5,10 +5,10 @@
   1
   
 hvm
-/usr/share/OVMF/OVMF_CODE.fd
 
   
 
+/usr/share/OVMF/OVMF_CODE.fd
   
   
 
-- 
2.39.2



[libvirt PATCH 2/2] conf: Fix migration in some firmware autoselection scenarios

2023-04-11 Thread Andrea Bolognani
Introduce a small kludge in the parser to avoid unnecessarily
blocking incoming migration from a range of recent libvirt
releases.

https://bugzilla.redhat.com/show_bug.cgi?id=2184966

Signed-off-by: Andrea Bolognani 
---
 src/conf/domain_conf.c| 39 ++-
 ...are-manual-efi-features.x86_64-latest.args | 35 +
 tests/qemuxml2argvtest.c  |  6 ++-
 ...ware-manual-efi-features.x86_64-latest.xml | 36 +
 tests/qemuxml2xmltest.c   |  1 +
 5 files changed, 114 insertions(+), 3 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
 create mode 100644 
tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 70e4d52ee6..bc20acf103 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17210,11 +17210,13 @@ virDomainDefParseBootKernelOptions(virDomainDef *def,
 
 static int
 virDomainDefParseBootFirmwareOptions(virDomainDef *def,
- xmlXPathContextPtr ctxt)
+ xmlXPathContextPtr ctxt,
+ unsigned int flags)
 {
 g_autofree char *firmware = virXPathString("string(./os/@firmware)", ctxt);
 g_autofree xmlNodePtr *nodes = NULL;
 g_autofree int *features = NULL;
+bool abiUpdate = !!(flags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
 int fw = 0;
 int n = 0;
 size_t i;
@@ -17222,6 +17224,39 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def,
 if ((n = virXPathNodeSet("./os/firmware/feature", ctxt, )) < 0)
 return -1;
 
+/* Migration compatibility kludge.
+ *
+ * Between 8.6.0 and 9.1.0 (extremes included), the migratable
+ * XML produced when feature-based firmware autoselection was
+ * enabled looked like
+ *
+ *   
+ * 
+ *   
+ *
+ * Notice how there's no firmware='foo' attribute for the 
+ * element, meaning that firmware autoselection is disabled, and
+ * yet some  elements, which are used to control the
+ * firmware autoselection process, are present. We don't consider
+ * this to be a valid combination, and want such a configuration
+ * to get rejected when submitted by users.
+ *
+ * In order to achieve that, while at the same time keeping
+ * migration coming from the libvirt versions listed above
+ * working, we can simply stop parsing early and ignore the
+ *  tags when firmware autoselection is not enabled,
+ * *except* if we're defining a new domain.
+ *
+ * This is safe to do because the configuration will either come
+ * from another libvirt instance, in which case it will have a
+ * properly filled in  element that contains enough
+ * information to successfully define and start the domain, or it
+ * will be a random configuration that lacks such information, in
+ * which case a different failure will be reported anyway.
+ */
+if (n > 0 && !firmware && !abiUpdate)
+return 0;
+
 if (n > 0)
 features = g_new0(int, VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST);
 
@@ -17350,7 +17385,7 @@ virDomainDefParseBootOptions(virDomainDef *def,
 case VIR_DOMAIN_OSTYPE_HVM:
 virDomainDefParseBootKernelOptions(def, ctxt);
 
-if (virDomainDefParseBootFirmwareOptions(def, ctxt) < 0)
+if (virDomainDefParseBootFirmwareOptions(def, ctxt, flags) < 0)
 return -1;
 
 if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0)
diff --git 
a/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args 
b/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
new file mode 100644
index 00..57c71542cd
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object 
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}'
 \
+-blockdev 
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
 \
+-blockdev 
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
 \
+-blockdev 
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
 \
+-blockdev 

[libvirt PATCH 0/2] conf: Fix migration in some firmware autoselection scenarios

2023-04-11 Thread Andrea Bolognani
See commit 2/2 for details.

Andrea Bolognani (2):
  tests: Tweak input file
  conf: Fix migration in some firmware autoselection scenarios

 src/conf/domain_conf.c| 39 ++-
 ...are-manual-efi-features.x86_64-latest.args | 35 +
 .../firmware-manual-efi-features.xml  |  2 +-
 tests/qemuxml2argvtest.c  |  6 ++-
 ...ware-manual-efi-features.x86_64-latest.xml | 36 +
 tests/qemuxml2xmltest.c   |  1 +
 6 files changed, 115 insertions(+), 4 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
 create mode 100644 
tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml

-- 
2.39.2



[PATCH] qemu: Fix potential crash during driver cleanup

2023-04-11 Thread Jim Fehlig
During qemu driver shutdown, objects are freed in qemuStateCleanup that
could still be used by active worker threads, resulting in crashes. E.g.
a worker thread could be processing a monitor EOF event after the
security manager is already disposed

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x7fd9a9a1e1fe in virSecurityManagerMoveImageMetadata 
(mgr=0x7fd948012160, pid=-1, src=src@entry=0x7fd98c072c90, dst=dst@entry=0x0)
at ../../src/security/security_manager.c:468
#1  0x7fd9646ff0f0 in qemuSecurityMoveImageMetadata 
(driver=driver@entry=0x7fd948043830, vm=vm@entry=0x7fd98c066db0, 
src=src@entry=0x7fd98c072c90,
dst=dst@entry=0x0) at ../../src/qemu/qemu_security.c:182
#2  0x7fd96462c7b0 in qemuBlockRemoveImageMetadata 
(driver=driver@entry=0x7fd948043830, vm=vm@entry=0x7fd98c066db0, 
diskTarget=0x7fd98c072530 "vda",
src=) at ../../src/qemu/qemu_block.c:2628
#3  0x7fd9646929d6 in qemuProcessStop (driver=driver@entry=0x7fd948043830, 
vm=vm@entry=0x7fd98c066db0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN,
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=) at 
../../src/qemu/qemu_process.c:7585
#4  0x7fd9646fc842 in processMonitorEOFEvent (vm=0x7fd98c066db0, 
driver=0x7fd948043830) at ../../src/qemu/qemu_driver.c:4794
#5  qemuProcessEventHandler (data=0x561a93febb60, opaque=0x7fd948043830) at 
../../src/qemu/qemu_driver.c:4900
#6  0x7fd9a9971a31 in virThreadPoolWorker 
(opaque=opaque@entry=0x561a93fb58e0) at ../../src/util/virthreadpool.c:163
(gdb) p mgr->drv
$2 = (virSecurityDriverPtr) 0x0

Prior to commit 7cf76d4e3ab, the worker thread pool was freed before
disposing any driver objects. Let's return to that pattern, but leave
the other changes made by 7cf76d4e3ab.

Signed-off-by: Jim Fehlig 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b4fb7ec1df..28e470e4a2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1062,6 +1062,7 @@ qemuStateCleanup(void)
 if (!qemu_driver)
 return -1;
 
+virThreadPoolFree(qemu_driver->workerPool);
 virObjectUnref(qemu_driver->migrationErrors);
 virLockManagerPluginUnref(qemu_driver->lockManager);
 virSysinfoDefFree(qemu_driver->hostsysinfo);
@@ -1078,7 +1079,6 @@ qemuStateCleanup(void)
 ebtablesContextFree(qemu_driver->ebtables);
 VIR_FREE(qemu_driver->qemuImgBinary);
 virObjectUnref(qemu_driver->domains);
-virThreadPoolFree(qemu_driver->workerPool);
 
 if (qemu_driver->lockFD != -1)
 virPidFileRelease(qemu_driver->config->stateDir, "driver", 
qemu_driver->lockFD);
-- 
2.40.0



Re: [PATCH 1/4] conf: add loader type 'none'

2023-04-11 Thread Gerd Hoffmann
  Hi,

> The second approach is the one described in the Ubuntu wiki[3], and
> also requires passing two files to QEMU, except this time they come
> from the opensbi and u-boot-qemu packages respectively. The usage
> looks like
> 
>   -bios /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.elf
>   -kernel /usr/lib/u-boot/qemu-riscv64_smode/uboot.elf
> 
> I think in this case the first file is a minimal build of OpenSBI
> that likely just initializes enough hardware before handing control
> to an arbitrary payload - in this case, u-boot.

Yes.  These days opensbi seems to be loaded by default, so the first
line is not needed.  In fact I'm running a guest here, on fedora 37 +
virt-preview with just this ...

  
hvm

/home/kraxel/projects/u-boot/build-qemu-riscv64-smode/u-boot.bin

  

... and it works fine.

There is also this variant ...

  


  

... to boot edk2 firmware.  Note this is a single image carrying both
code and vars.  Also note 'index=1', which I think is needed because the
(default) opensbi is loaded to the pflash device with 'index=0'.

This doesn't boot the distro due to grub2 not having full riscv64 efi
support (yet).

take care,
  Gerd



Re: [libvirt PATCH 2/3] meson: drop explicit python interpreter

2023-04-11 Thread Michal Prívozník
On 4/11/23 09:27, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Apr 11, 2023 at 11:10 AM Michal Prívozník  > wrote:
> 
> On 4/6/23 17:58, marcandre.lur...@redhat.com
>  wrote:
> > From: Marc-André Lureau  >
> >
> > meson wraps python scripts already on win32, so we end up with these
> > failing commands:
> >
> > [1/359] "C:/msys64/ucrt64/bin/meson" "--internal" "exe"
> "--capture" "src/util/virkeycodetable_atset1.h" "--" "sh"
> "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh"
> "C:/msys64/ucrt64/bin/python3.EXE" "python"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen"
> "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv"
> "atset1"
> > FAILED: src/util/virkeycodetable_atset1.h
> > "C:/msys64/ucrt64/bin/meson" "--internal" "exe" "--capture"
> "src/util/virkeycodetable_atset1.h" "--" "sh"
> "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh"
> "C:/msys64/ucrt64/bin/python3.EXE" "python"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen"
> "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv"
> "atset1"
> >
> > If LC_ALL, LANG and LC_CTYPE need to be set, it would probably be
> better
> > to use a meson environment() instead.
> >
> > Signed-off-by: Marc-André Lureau  >
> > ---
> >  docs/manpages/meson.build | 4 ++--
> >  docs/meson.build          | 6 ++
> >  src/admin/meson.build     | 4 ++--
> >  src/esx/meson.build       | 4 ++--
> >  src/hyperv/meson.build    | 2 +-
> >  src/meson.build           | 8 
> >  src/util/meson.build      | 4 ++--
> >  7 files changed, 15 insertions(+), 17 deletions(-)
> 
> After this, there are still some occurrences of meson_python_prog or
> python3_prog left; mostly in locations which are never built on Windows
> (e.g. src/qemu/, src/network/ and so on. But is it worth removing
> them too?
> 
> 
> If we don't have a good reason for this extra wrapping, yes. Is gitlab
> CI covering enough to validate the change? Someone more familiar with
> libvirt build environments should know better.

Yeah, good point. Let me run this as-is before pushing it:

https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/833467320

and actually, it breaks the build. Thing is, it looks for scripts in the
builddir rather than srcdir:

[4/1482] /usr/bin/meson --internal exe --capture 
src/util/virkeycodetable_atset1.h -- 
/builds/MichalPrivoznik/libvirt/rpmbuild/BUILD/libvirt-9.3.0/src/keycodemapdb/tools/keymap-gen
 code-table --lang stdc --varname virKeyCodeTable_atset1 
/builds/MichalPrivoznik/libvirt/rpmbuild/BUILD/libvirt-9.3.0/src/keycodemapdb/data/keymaps.csv
 atset1 

But what I don't understand is: why prefixing the script with
meson_python_prog and/or python3_prog causes the script to be looked for
in the srcdir.

Michal



Re: [libvirt PATCH 3/3] rpc/ssh: ssh_userauth_agent() is not supported on win32

2023-04-11 Thread Marc-André Lureau
Hi

On Tue, Apr 11, 2023 at 11:10 AM Michal Prívozník 
wrote:

> On 4/6/23 17:58, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > The function does not exist on win32.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  src/rpc/virnetlibsshsession.c | 14 --
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/rpc/virnetlibsshsession.c
> b/src/rpc/virnetlibsshsession.c
> > index e71a79d0fb..e94b0d7a2e 100644
> > --- a/src/rpc/virnetlibsshsession.c
> > +++ b/src/rpc/virnetlibsshsession.c
> > @@ -60,7 +60,9 @@ typedef enum {
> >  VIR_NET_LIBSSH_AUTH_KEYBOARD_INTERACTIVE,
> >  VIR_NET_LIBSSH_AUTH_PASSWORD,
> >  VIR_NET_LIBSSH_AUTH_PRIVKEY,
> > -VIR_NET_LIBSSH_AUTH_AGENT
> > +#ifndef WIN32
> > +VIR_NET_LIBSSH_AUTH_AGENT,
> > +#endif
> >  } virNetLibsshAuthMethods;
> >
>
> I'd just drop this hunk, and ...
>
> >
> > @@ -698,6 +700,7 @@ virNetLibsshAuthenticate(virNetLibsshSession *sess)
> >  /* try to authenticate using the keyboard interactive way */
> >  ret = virNetLibsshAuthenticateKeyboardInteractive(sess,
> auth);
> >  break;
> > +#ifndef WIN32
> >  case VIR_NET_LIBSSH_AUTH_AGENT:
> >  /* try to authenticate using ssh-agent */
> >  ret = ssh_userauth_agent(sess->session, NULL);
> > @@ -708,6 +711,7 @@ virNetLibsshAuthenticate(virNetLibsshSession *sess)
> > errmsg);
> >  }
> >  break;
> > +#endif
>
>
> .. here just wrap the actual ssh_userauth_agent() call in ifdnef. The
> @ret is set to SSH_AUTH_DENIED beforehand, and later in the code a
> proper error message is reported.
>
> >  case VIR_NET_LIBSSH_AUTH_PRIVKEY:
> >  /* try to authenticate using the provided ssh key */
> >  ret = virNetLibsshAuthenticatePrivkey(sess, auth);
> > @@ -861,8 +865,13 @@
> virNetLibsshSessionAuthAddPasswordAuth(virNetLibsshSession *sess,
> >  }
> >
> >  int
> > -virNetLibsshSessionAuthAddAgentAuth(virNetLibsshSession *sess)
> > +virNetLibsshSessionAuthAddAgentAuth(virNetLibsshSession *sess
> G_GNUC_UNUSED)
> >  {
> > +#ifdef WIN32
> > +virReportError(VIR_ERR_LIBSSH, "%s",
> > +   _("Agent authentication is not supported on this
> host"));
> > +return -1;
> > +#else
> >  virNetLibsshAuthMethod *auth;
> >
> >  virObjectLock(sess);
> > @@ -873,6 +882,7 @@
> virNetLibsshSessionAuthAddAgentAuth(virNetLibsshSession *sess)
> >
> >  virObjectUnlock(sess);
> >  return 0;
> > +#endif
> >  }
> >
> >  int
>
> This hunk alone is enough to ensure agent is not available on WIN32.
>
>
Sure, that's fine with me.

thanks


Re: [libvirt PATCH 2/3] meson: drop explicit python interpreter

2023-04-11 Thread Marc-André Lureau
Hi

On Tue, Apr 11, 2023 at 11:10 AM Michal Prívozník 
wrote:

> On 4/6/23 17:58, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > meson wraps python scripts already on win32, so we end up with these
> > failing commands:
> >
> > [1/359] "C:/msys64/ucrt64/bin/meson" "--internal" "exe" "--capture"
> "src/util/virkeycodetable_atset1.h" "--" "sh"
> "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh"
> "C:/msys64/ucrt64/bin/python3.EXE" "python"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen"
> "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv"
> "atset1"
> > FAILED: src/util/virkeycodetable_atset1.h
> > "C:/msys64/ucrt64/bin/meson" "--internal" "exe" "--capture"
> "src/util/virkeycodetable_atset1.h" "--" "sh"
> "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh"
> "C:/msys64/ucrt64/bin/python3.EXE" "python"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen"
> "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1"
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv"
> "atset1"
> >
> > If LC_ALL, LANG and LC_CTYPE need to be set, it would probably be better
> > to use a meson environment() instead.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  docs/manpages/meson.build | 4 ++--
> >  docs/meson.build  | 6 ++
> >  src/admin/meson.build | 4 ++--
> >  src/esx/meson.build   | 4 ++--
> >  src/hyperv/meson.build| 2 +-
> >  src/meson.build   | 8 
> >  src/util/meson.build  | 4 ++--
> >  7 files changed, 15 insertions(+), 17 deletions(-)
>
> After this, there are still some occurrences of meson_python_prog or
> python3_prog left; mostly in locations which are never built on Windows
> (e.g. src/qemu/, src/network/ and so on. But is it worth removing them too?
>
>
If we don't have a good reason for this extra wrapping, yes. Is gitlab CI
covering enough to validate the change? Someone more familiar with libvirt
build environments should know better.

thanks


Re: [libvirt PATCH 2/3] meson: drop explicit python interpreter

2023-04-11 Thread Michal Prívozník
On 4/6/23 17:58, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> meson wraps python scripts already on win32, so we end up with these
> failing commands:
> 
> [1/359] "C:/msys64/ucrt64/bin/meson" "--internal" "exe" "--capture" 
> "src/util/virkeycodetable_atset1.h" "--" "sh" 
> "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh" 
> "C:/msys64/ucrt64/bin/python3.EXE" "python" 
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen" 
> "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1" 
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv" "atset1"
> FAILED: src/util/virkeycodetable_atset1.h
> "C:/msys64/ucrt64/bin/meson" "--internal" "exe" "--capture" 
> "src/util/virkeycodetable_atset1.h" "--" "sh" 
> "C:/msys64/home/marca/src/libvirt/scripts/meson-python.sh" 
> "C:/msys64/ucrt64/bin/python3.EXE" "python" 
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/tools/keymap-gen" 
> "code-table" "--lang" "stdc" "--varname" "virKeyCodeTable_atset1" 
> "C:/msys64/home/marca/src/libvirt/src/keycodemapdb/data/keymaps.csv" "atset1"
> 
> If LC_ALL, LANG and LC_CTYPE need to be set, it would probably be better
> to use a meson environment() instead.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  docs/manpages/meson.build | 4 ++--
>  docs/meson.build  | 6 ++
>  src/admin/meson.build | 4 ++--
>  src/esx/meson.build   | 4 ++--
>  src/hyperv/meson.build| 2 +-
>  src/meson.build   | 8 
>  src/util/meson.build  | 4 ++--
>  7 files changed, 15 insertions(+), 17 deletions(-)

After this, there are still some occurrences of meson_python_prog or
python3_prog left; mostly in locations which are never built on Windows
(e.g. src/qemu/, src/network/ and so on. But is it worth removing them too?

Michal



Re: [libvirt PATCH 3/3] rpc/ssh: ssh_userauth_agent() is not supported on win32

2023-04-11 Thread Michal Prívozník
On 4/6/23 17:58, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> The function does not exist on win32.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  src/rpc/virnetlibsshsession.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
> index e71a79d0fb..e94b0d7a2e 100644
> --- a/src/rpc/virnetlibsshsession.c
> +++ b/src/rpc/virnetlibsshsession.c
> @@ -60,7 +60,9 @@ typedef enum {
>  VIR_NET_LIBSSH_AUTH_KEYBOARD_INTERACTIVE,
>  VIR_NET_LIBSSH_AUTH_PASSWORD,
>  VIR_NET_LIBSSH_AUTH_PRIVKEY,
> -VIR_NET_LIBSSH_AUTH_AGENT
> +#ifndef WIN32
> +VIR_NET_LIBSSH_AUTH_AGENT,
> +#endif
>  } virNetLibsshAuthMethods;
>  

I'd just drop this hunk, and ...

>  
> @@ -698,6 +700,7 @@ virNetLibsshAuthenticate(virNetLibsshSession *sess)
>  /* try to authenticate using the keyboard interactive way */
>  ret = virNetLibsshAuthenticateKeyboardInteractive(sess, auth);
>  break;
> +#ifndef WIN32
>  case VIR_NET_LIBSSH_AUTH_AGENT:
>  /* try to authenticate using ssh-agent */
>  ret = ssh_userauth_agent(sess->session, NULL);
> @@ -708,6 +711,7 @@ virNetLibsshAuthenticate(virNetLibsshSession *sess)
> errmsg);
>  }
>  break;
> +#endif


.. here just wrap the actual ssh_userauth_agent() call in ifdnef. The
@ret is set to SSH_AUTH_DENIED beforehand, and later in the code a
proper error message is reported.

>  case VIR_NET_LIBSSH_AUTH_PRIVKEY:
>  /* try to authenticate using the provided ssh key */
>  ret = virNetLibsshAuthenticatePrivkey(sess, auth);
> @@ -861,8 +865,13 @@ 
> virNetLibsshSessionAuthAddPasswordAuth(virNetLibsshSession *sess,
>  }
>  
>  int
> -virNetLibsshSessionAuthAddAgentAuth(virNetLibsshSession *sess)
> +virNetLibsshSessionAuthAddAgentAuth(virNetLibsshSession *sess G_GNUC_UNUSED)
>  {
> +#ifdef WIN32
> +virReportError(VIR_ERR_LIBSSH, "%s",
> +   _("Agent authentication is not supported on this host"));
> +return -1;
> +#else
>  virNetLibsshAuthMethod *auth;
>  
>  virObjectLock(sess);
> @@ -873,6 +882,7 @@ virNetLibsshSessionAuthAddAgentAuth(virNetLibsshSession 
> *sess)
>  
>  virObjectUnlock(sess);
>  return 0;
> +#endif
>  }
>  
>  int

This hunk alone is enough to ensure agent is not available on WIN32.

Michal



Re: [libvirt PATCH 0/3] RFC: fix compilation on msys2

2023-04-11 Thread Michal Prívozník
On 4/6/23 17:58, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> Hi,
> 
> libvirt fails to compile on msys2/win32. Here is a few patches that solve it,
> but the way python scripts are being handled in general is a bit odd, so this 
> is
> RFC. (fwiw, a lot of tests fail though, I can send the log if anyone is
> interested - or attach it to a gitlab issue)
> 
> Fixes:
> https://gitlab.com/libvirt/libvirt/-/issues/453
> 
> Marc-André Lureau (3):
>   meson: don't look for unix paths on win32
>   meson: drop explicit python interpreter
>   rpc/ssh: ssh_userauth_agent() is not supported on win32
> 
>  docs/manpages/meson.build |  4 ++--
>  docs/meson.build  |  6 ++
>  meson.build   | 14 +-
>  src/admin/meson.build |  4 ++--
>  src/esx/meson.build   |  4 ++--
>  src/hyperv/meson.build|  2 +-
>  src/meson.build   |  8 
>  src/rpc/virnetlibsshsession.c | 14 --
>  src/util/meson.build  |  4 ++--
>  9 files changed, 36 insertions(+), 24 deletions(-)
> 

Hey, thanks for this. I have couple of questions though. No need to
resend, I can squash in obvious changes if you agree before pushing.

Michal



Re: [PATCH 00/21] qemu capability testing cleanups and improvements (part 5)

2023-04-11 Thread Ján Tomko

On a Friday in 2023, Peter Krempa wrote:

This series applies on top of 'part 4' fetch everything from my repo:

git fetch https://gitlab.com/pipo.sk/libvirt.git aarch-send

In this part tests for the 'aarch64' platform are converted to use real
capabilities.

Peter Krempa (21):
 virDomainPCIAddressSetExtensionAlloc: Remove return value
 qemuxml2argvdata: Do not symlink output files for aarch64 gic tests
 qemuxml2argvtest: Use real capabilities in tests for picking the
   aarch64 GIC version
 qemuxml2argvtest: Convert DO_TEST_GIC to use real latest capabilities
 qemuxml2argvtest: Convert the rest of GIC tests to latest capabilities
 qemuxml2argvtest: Add real-caps versions of 'aarch64-virt-virtio'
 qemuxml2argvtest: Drop "aarch64-virt-2.6-virtio-pci-default" case
 qemuxml2argv: Test default aarch64 cofig without PCIe support
 qemuxml2argvtest: Modernize 'balloon-mmio-deflate'
 qemuxml2argvtest: Don't symlink output files for 'mach-virt-' cases
 qemuxml2argvtest: Modernize all 'mach-virt-' aarch64 test cases
 qemuxml2argvtest: Update 'aarch64-virtio-pci-manual-addresses' case
 qemuxml2*test: Drop fake-caps invocation of
   'aarch64-virtio-pci-manual-addresses'
 qemuxml2(argv|xml)test: Modernize testing of USB controllers on
   aarch64
 qemuxml2argvtest: Modernize the rest of 'aarch64' cases
 qemuxml2xmlout: Do not symlink output files for 'aarch64-gic' cases
 qemuxml2xmltest: Modernize 'aarch64-gic*' test cases
 qemuxml2xmloutdata: Don't symlink output data for 'mach-virt*' cases
 qemuxml2xmltest: Modernize 'mach-virt*' cases
 qemuxml2xmltest: Convert rest of 'aarch64' cases to real capabilities
 testutilsqemu: Drop fake capability testing infrastructure for
   'aarch64'

src/conf/domain_addr.c|  13 +-
...h64-aavmf-virtio-mmio.aarch64-latest.args} |  20 +-
...rch64-cpu-passthrough.aarch64-latest.args} |  26 +--

[..]

tests/qemuxml2xmltest.c   | 153 ++--
tests/testutilsqemu.c |   6 -
94 files changed, 1396 insertions(+), 557 deletions(-)
rename tests/qemuxml2argvdata/{aarch64-aavmf-virtio-mmio.args => 
aarch64-aavmf-virtio-mmio.aarch64-latest.args} (57%)
rename tests/qemuxml2argvdata/{aarch64-virtio-pci-manual-addresses.args => 
aarch64-cpu-passthrough.aarch64-latest.args} (52%)
mode change 12 => 100644 
tests/qemuxml2argvdata/aarch64-gic-default-both.args
mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-default-v2.args
mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-default-v3.args

[..]

rename tests/qemuxml2xmloutdata/{mach-virt-serial-pci.xml => 
mach-virt-serial-pci.aarch64-latest.xml} (71%)
rename tests/qemuxml2xmloutdata/{mach-virt-serial-usb.xml => 
mach-virt-serial-usb.aarch64-latest.xml} (93%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCHv2] storage_file_probe: change maximum len value in vmdk4GetBackingStore

2023-04-11 Thread Ján Tomko

On a Monday in 2023, Anastasia Belova wrote:

desc length should be always less than VIR_STORAGE_MAX_HEADER.
If len = VIR_STORAGE_MAX_HEADER, desc may be out of bounds.

Fixes: 348b4e254b ("storage: always probe type with buffer")
Signed-off-by: Anastasia Belova 
---
v2: change Fixes: line



Oops, I already pushed it as:
commit 2c6b5a84257379e516ca1999782dca88dfd8a9de
but forgot to change the commit line.

Jano


src/storage_file/storage_file_probe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)


signature.asc
Description: PGP signature