Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Thomas Huth

On 17/02/2023 17.38, Paolo Bonzini wrote:

On 2/17/23 11:47, Daniel P. Berrangé wrote:

On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:

I feel the discussion petered out without a conclusion.

I don't think letting the status quo win by inertia is a good outcome
here.

Which 32-bit hosts are still useful, and why?


Which 32-bit hosts does Linux still provide KVM  support for.


All except ARM: MIPS, x86, PPC and RISC-V.

I would like to remove x86, but encountered some objections.


So if I got that right:


https://lore.kernel.org/all/b8fa9561295bb6af2b7fcaa8125c6a3b89b305c7.ca...@redhat.com/

... the objection is mainly that some kernel developers want to keep the 
code around for easier testing of nested 32-bit guests L1 hypervisors.


If that's the only use case that is still around for the 32-bit KVM x86 
kernel code, I guess it should also be fine to use older versions of QEMU in 
those L1 hypervisor guests (assuming you have to use an older 32-bit Linux 
distro for this anyway).


So unless I got that wrong, there is really nobody around anymore who needs 
an *upstream* QEMU for running 32-bit x86 KVM hosts, is there?


Please correct me if I'm wrong, but I think we can really deprecated at 
least qemu-system-i386 and qemu-system-arm now, can't we?


 Thanks,
  Thomas



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Thomas Huth

On 17/02/2023 18.43, Philippe Mathieu-Daudé wrote:

(Cc'ing Huacai & Jiaxun).

On 17/2/23 17:38, Paolo Bonzini wrote:

On 2/17/23 11:47, Daniel P. Berrangé wrote:

On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:

I feel the discussion petered out without a conclusion.

I don't think letting the status quo win by inertia is a good outcome
here.

Which 32-bit hosts are still useful, and why?


Which 32-bit hosts does Linux still provide KVM  support for.


All except ARM: MIPS, x86, PPC and RISC-V.

I would like to remove x86, but encountered some objections.

MIPS, nobody is really using it I think.


32-bit was added in 2014, commit 222e7d11e7 ("target-mips: Enable KVM
support in build system"). I'm not aware of anybody using it (even
testing it). I don't have hardware to test it (neither time).


Could you maybe suggest a kernel patch to remove it, to see what happens? 
... if nobody objects to the removal of the 32-bit MIPS KVM kernel support 
and the patch gets merged, that would help us in the long run, I think.


 Thanks,
  Thomas



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Richard Henderson

On 2/17/23 06:06, Reinoud Zandijk wrote:

On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:

I feel the discussion petered out without a conclusion.

I don't think letting the status quo win by inertia is a good outcome
here.

Which 32-bit hosts are still useful, and why?


NetBSD runs on a bunch of 32 bit-only hosts (sparc32, ppc32, armv7, vax,
mips32 etc.) that all run Qemu fine. They are all actively maintained and
released as part of the main releases.


Are you sure about that?  TCG doesn't support sparc32 or vax.
I suppose you could be using TCI, but I can't even imagine how
slow that would be on vax.


r~



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Philippe Mathieu-Daudé

(Cc'ing Huacai & Jiaxun).

On 17/2/23 17:38, Paolo Bonzini wrote:

On 2/17/23 11:47, Daniel P. Berrangé wrote:

On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:

I feel the discussion petered out without a conclusion.

I don't think letting the status quo win by inertia is a good outcome
here.

Which 32-bit hosts are still useful, and why?


Which 32-bit hosts does Linux still provide KVM  support for.


All except ARM: MIPS, x86, PPC and RISC-V.

I would like to remove x86, but encountered some objections.

MIPS, nobody is really using it I think.


32-bit was added in 2014, commit 222e7d11e7 ("target-mips: Enable KVM
support in build system"). I'm not aware of anybody using it (even
testing it). I don't have hardware to test it (neither time).
We are still cross-compiling it although.

64-bit support was added recently (see commit aa2953fd16 "configure:
Add KVM target support for MIPS64") and is used (see commit fbc5884ce2
"meson.build: Re-enable KVM support for MIPS" from 2020), however I
tend to see it more as hobbyist use than production one. Besides it
is listed as 'Odd Fixes' in MAINTAINERS (still 2020, commit 134f7f7da1
"MAINTAINERS: Reactivate MIPS KVM CPUs").


So that leaves PPC and RISC-V.


If any, is there an EOL date for Linux 32-bit KVM support ?


No, and I don't think there's going to be one.

Paolo





Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Daniel P . Berrangé
On Fri, Feb 17, 2023 at 05:06:42PM +0100, Reinoud Zandijk wrote:
> On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
> > I feel the discussion petered out without a conclusion.
> > 
> > I don't think letting the status quo win by inertia is a good outcome
> > here.
> > 
> > Which 32-bit hosts are still useful, and why?
> 
> NetBSD runs on a bunch of 32 bit-only hosts (sparc32, ppc32, armv7, vax,
> mips32 etc.) that all run Qemu fine. They are all actively maintained and
> released as part of the main releases.
> 
> Maintaining 32 bit host support is thus vital for those machines; not everyone
> runs a 64 bit system.

In the context of NetBSD, is QEMU a leaf node package, or is it a
dependancy of anything involved in delivery of the distro ?

ie, if QEMU were to drop 32-bit support, if it is merely a leaf node
package, then end users of NetBSD would not have access to QEMU, but
you would still be able to deliver the rest of NetBSD on those platforms
without the QEMU package present ?

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Reinoud Zandijk
On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
> I feel the discussion petered out without a conclusion.
> 
> I don't think letting the status quo win by inertia is a good outcome
> here.
> 
> Which 32-bit hosts are still useful, and why?

NetBSD runs on a bunch of 32 bit-only hosts (sparc32, ppc32, armv7, vax,
mips32 etc.) that all run Qemu fine. They are all actively maintained and
released as part of the main releases.

Maintaining 32 bit host support is thus vital for those machines; not everyone
runs a 64 bit system.

with regards,
Reinoud Zandijk,
NetBSD developer.



signature.asc
Description: PGP signature


[libvirt PATCH v2] qemu: implement QEMU NBD source reconnect delay attribute

2023-02-17 Thread Christian Nautze
Currently it's only possible to set this parameter during domain
creation via QEMU commandline passthrough feature.
With the new delay attribute it's also possible to set this
parameter if you want to attach a new NBD disk
using "virsh attach-device domain device.xml" e.g.:

  


  
  


  

Signed-off-by: Christian Nautze 

---
Change: reconnect element: drop mandatory 'enabled' attribute when using 'delay'
---
 docs/formatdomain.rst | 11 +++--
 src/conf/domain_conf.c| 12 ++
 src/conf/schemas/domaincommon.rng |  3 +++
 src/conf/schemas/storagecommon.rng| 13 ---
 src/conf/storage_source_conf.c|  1 +
 src/conf/storage_source_conf.h|  4 
 src/qemu/qemu_block.c |  4 +++-
 src/qemu/qemu_domain.c|  9 
 .../disk-network-nbd.x86_64-latest.args   | 23 +++
 tests/qemuxml2argvdata/disk-network-nbd.xml   |  8 +++
 tests/qemuxml2xmloutdata/disk-network-nbd.xml |  9 
 11 files changed, 81 insertions(+), 16 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 36c6d87907..ee30c51cea 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2946,13 +2946,20 @@ paravirtualized driver is specified via the ``disk`` 
element.
   are intended to be default, then the entire element may be omitted.
``reconnect``
   For disk type ``vhostuser`` configures reconnect timeout if the 
connection
-  is lost. It has two mandatory attributes:
+  is lost. This is set with the two mandatory attributes ``enabled`` and
+  ``timeout``.
+  For disk type ``network`` and protocol ``nbd`` the QEMU NBD reconnect 
delay
+  can be set via attribute ``delay``:
 
   ``enabled``
  If the reconnect feature is enabled, accepts ``yes`` and ``no``
   ``timeout``
  The amount of seconds after which hypervisor tries to reconnect.
-
+  ``delay``
+ Only for NBD hosts. The amount of seconds during which all requests 
are
+ paused and will be rerun after a successful reconnect. After that 
time, any
+ delayed requests and all future requests before a successful reconnect
+ will immediately fail. If not set the default QEMU value is 0.
 
For a "file" or "volume" disk type which represents a cdrom or floppy (the
``device`` attribute), it is possible to define policy what to do with the
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a5578324b9..3f2ba2aab8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7146,6 +7146,15 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
 src->tlsFromConfig = !!value;
 }
 
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NBD) {
+xmlNodePtr cur;
+if ((cur = virXPathNode("./reconnect", ctxt))) {
+if (virXMLPropUInt(cur, "delay", 10, VIR_XML_PROP_NONE,
+   >reconnectDelay) < 0)
+return -1;
+}
+}
+
 /* for historical reasons we store the volume and image name in one XML
  * element although it complicates thing when attempting to access them. */
 if (src->path &&
@@ -22073,6 +22082,9 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
 virBufferAddLit(childBuf, "/>\n");
 }
 
+if (src->reconnectDelay) {
+virBufferAsprintf(childBuf, "\n", 
src->reconnectDelay);
+}
 
 virBufferEscapeString(childBuf, "\n", src->snapshot);
 virBufferEscapeString(childBuf, "\n", src->configFile);
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index a57dd212ab..dd5fbeac09 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2199,6 +2199,9 @@
 
 
 
+
+  
+
 
   
 
diff --git a/src/conf/schemas/storagecommon.rng 
b/src/conf/schemas/storagecommon.rng
index 4d6e646c9a..582375358c 100644
--- a/src/conf/schemas/storagecommon.rng
+++ b/src/conf/schemas/storagecommon.rng
@@ -55,14 +55,21 @@
 
   
 
-  
-
-  
+  
+
+  
+
+  
   
 
   
 
   
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index cecd7e811e..58009fd06e 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -811,6 +811,7 @@ virStorageSourceCopy(const virStorageSource *src,
 def->sslverify = src->sslverify;
 def->readahead = src->readahead;
 def->timeout = src->timeout;
+def->reconnectDelay = src->reconnectDelay;
 def->metadataCacheMaxSize = src->metadataCacheMaxSize;
 
 /* storage driver metadata are not copied */
diff --git 

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Paolo Bonzini

On 2/17/23 11:47, Daniel P. Berrangé wrote:

On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:

I feel the discussion petered out without a conclusion.

I don't think letting the status quo win by inertia is a good outcome
here.

Which 32-bit hosts are still useful, and why?


Which 32-bit hosts does Linux still provide KVM  support for.


All except ARM: MIPS, x86, PPC and RISC-V.

I would like to remove x86, but encountered some objections.

MIPS, nobody is really using it I think.

So that leaves PPC and RISC-V.


If any, is there an EOL date for Linux 32-bit KVM support ?


No, and I don't think there's going to be one.

Paolo



[PATCH] docs: ACL: Mention the ACL object name along with the corresponding libvirt object name

2023-02-17 Thread Peter Krempa
It's not trivial to figure out the ACL object name from our
documentation. Add it above the table outlining existing permissions.

Signed-off-by: Peter Krempa 
---
 scripts/genaclperms.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/genaclperms.py b/scripts/genaclperms.py
index e228b3ef60..e635dae50b 100755
--- a/scripts/genaclperms.py
+++ b/scripts/genaclperms.py
@@ -91,6 +91,7 @@ for obj in sorted(perms.keys()):
 olink = "object_" + obj.lower()

 print('%s' % (olink, klass))
+print('The %s libvirt object is represented as 
%s.' % (klass, obj.lower()))
 print('')
 print('  ')
 print('')
-- 
2.39.1



Re: [PATCH] docs: ACL: Show which permissions are allowed for unauthenticated connections

2023-02-17 Thread Daniel P . Berrangé
On Fri, Feb 17, 2023 at 04:33:12PM +0100, Peter Krempa wrote:
> Certain APIs are allowed also without authentication but the ACL page
> didn't outline which. Generate a new column with the information.
> 
> Signed-off-by: Peter Krempa 
> ---
>  docs/acl.html.in   | 3 ++-
>  scripts/genaclperms.py | 7 +++
>  2 files changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé 


> 
> diff --git a/docs/acl.html.in b/docs/acl.html.in
> index 3d0f651864..268d3aebd3 100644
> --- a/docs/acl.html.in
> +++ b/docs/acl.html.in
> @@ -20,7 +20,8 @@
>state, where the only API operations allowed are those required
>to complete authentication. After successful authentication, a
>connection either has full, unrestricted access to all libvirt
> -  API calls, or is locked down to only "read only" operations,
> +  API calls, or is locked down to only "read only" (see 'Anonymous'
> +  in the table below) operations,
>according to what socket a client connection originated on.
>  
> 
> diff --git a/scripts/genaclperms.py b/scripts/genaclperms.py
> index e228b3ef60..43616dad04 100755
> --- a/scripts/genaclperms.py
> +++ b/scripts/genaclperms.py
> @@ -96,6 +96,7 @@ for obj in sorted(perms.keys()):
>  print('')
>  print('  Permission')
>  print('  Description')
> +print('  Anonymous')
>  print('')
>  print('  ')
>  print('  ')
> @@ -103,6 +104,11 @@ for obj in sorted(perms.keys()):
>  for perm in sorted(perms[obj].keys()):
>  description = perms[obj][perm]["desc"]
> 
> +if perms[obj][perm]["anonymous"]:
> +anonymous = 'yes'
> +else:
> +anonymous = ''
> +
>  if description is None:
>  raise Exception("missing description for %s.%s" % (obj, perm))
> 
> @@ -112,6 +118,7 @@ for obj in sorted(perms.keys()):
>  print('')
>  print('  %s' % (plink, perm))
>  print('  %s' % description)
> +print('  %s' % anonymous)
>  print('')
> 
>  print('  ')
> -- 
> 2.39.1
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[PATCH] docs: ACL: Show which permissions are allowed for unauthenticated connections

2023-02-17 Thread Peter Krempa
Certain APIs are allowed also without authentication but the ACL page
didn't outline which. Generate a new column with the information.

Signed-off-by: Peter Krempa 
---
 docs/acl.html.in   | 3 ++-
 scripts/genaclperms.py | 7 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/acl.html.in b/docs/acl.html.in
index 3d0f651864..268d3aebd3 100644
--- a/docs/acl.html.in
+++ b/docs/acl.html.in
@@ -20,7 +20,8 @@
   state, where the only API operations allowed are those required
   to complete authentication. After successful authentication, a
   connection either has full, unrestricted access to all libvirt
-  API calls, or is locked down to only "read only" operations,
+  API calls, or is locked down to only "read only" (see 'Anonymous'
+  in the table below) operations,
   according to what socket a client connection originated on.
 

diff --git a/scripts/genaclperms.py b/scripts/genaclperms.py
index e228b3ef60..43616dad04 100755
--- a/scripts/genaclperms.py
+++ b/scripts/genaclperms.py
@@ -96,6 +96,7 @@ for obj in sorted(perms.keys()):
 print('')
 print('  Permission')
 print('  Description')
+print('  Anonymous')
 print('')
 print('  ')
 print('  ')
@@ -103,6 +104,11 @@ for obj in sorted(perms.keys()):
 for perm in sorted(perms[obj].keys()):
 description = perms[obj][perm]["desc"]

+if perms[obj][perm]["anonymous"]:
+anonymous = 'yes'
+else:
+anonymous = ''
+
 if description is None:
 raise Exception("missing description for %s.%s" % (obj, perm))

@@ -112,6 +118,7 @@ for obj in sorted(perms.keys()):
 print('')
 print('  %s' % (plink, perm))
 print('  %s' % description)
+print('  %s' % anonymous)
 print('')

 print('  ')
-- 
2.39.1



Re: [PATCH 2/2] libvirt-nodedev: Allow read-only access to virNodeDeviceGetAutostart

2023-02-17 Thread Daniel P . Berrangé
On Fri, Feb 17, 2023 at 04:11:11PM +0100, Peter Krempa wrote:
> Fetching whether a node-device is marked for autostart can be allowed
> from read-only connections similarly to other objects.
> 
> Fixes: c6607a25b93
> Signed-off-by: Peter Krempa 
> ---
>  src/libvirt-nodedev.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [PATCH 1/2] access: Allow 'node-device.read' permission for anonymous users

2023-02-17 Thread Daniel P . Berrangé
On Fri, Feb 17, 2023 at 04:11:10PM +0100, Peter Krempa wrote:
> For all other objects we allow the 'read' permission for anonymous
> users. In fact the idea is to allow all permissions users using the
> readonly connection would have.
> 
> This impacts the following APIs (in terms of RPC procedure names):
> 
>   $ git grep -A 3 node_device:read | grep REMOTE
>   src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_GET_XML_DESC = 114,
>   src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_GET_PARENT = 115,
>   src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_NUM_OF_CAPS = 116,
>   src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_LIST_CAPS = 117,
>   src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_GET_AUTOSTART = 
> 433,
>   src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_IS_PERSISTENT = 
> 435,
>   src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_IS_ACTIVE = 436,
> 
> Fixes: a93cd08f
> Signed-off-by: Peter Krempa 
> ---
>  src/access/viraccessperm.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Daniel P. Berrangé 


> 
> diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
> index 051246a7b6..2f04459ed9 100644
> --- a/src/access/viraccessperm.h
> +++ b/src/access/viraccessperm.h
> @@ -473,6 +473,7 @@ typedef enum {
>  /**
>   * @desc: Read node device
>   * @message: Reading node device configuration requires authorization
> + * @anonymous: 1
>   */
>  VIR_ACCESS_PERM_NODE_DEVICE_READ,
> 
> -- 
> 2.39.1
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[PATCH v2 1/2] selinux: Swap two blocks handling setfilecon_raw() failure

2023-02-17 Thread Michal Privoznik
In virSecuritySELinuxSetFileconImpl() we have code that handles
setfilecon_raw() failure. The code consists of two blocks: one
for dealing with shared filesystem like NFS (errno is ENOTSUP or
EROFS) and the other block that's dealing with EPERM for
privileged daemon. Well, the order of these two blocks is a bit
confusing because the comment above them mentions the NFS case
but EPERM block follows. Swap these two blocks to make it less
confusing.

Signed-off-by: Michal Privoznik 
---
 src/security/security_selinux.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 4d4a1705e6..e9c4051a98 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1261,22 +1261,9 @@ virSecuritySELinuxSetFileconImpl(const char *path,
  * boolean tunables to allow it ...
  */
 VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
-if (setfilecon_errno != EOPNOTSUPP && setfilecon_errno != ENOTSUP &&
-setfilecon_errno != EROFS) {
+if (setfilecon_errno == EOPNOTSUPP || setfilecon_errno == ENOTSUP ||
+setfilecon_errno == EROFS) {
 VIR_WARNINGS_RESET
-/* However, don't claim error if SELinux is in Enforcing mode and
- * we are running as unprivileged user and we really did see EPERM.
- * Otherwise we want to return error if SELinux is Enforcing. */
-if (security_getenforce() == 1 &&
-(setfilecon_errno != EPERM || privileged)) {
-virReportSystemError(setfilecon_errno,
- _("unable to set security context '%s' on 
'%s'"),
- tcon, path);
-return -1;
-}
-VIR_WARN("unable to set security context '%s' on '%s' (errno %d)",
- tcon, path, setfilecon_errno);
-} else {
 const char *msg;
 if (virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 1 &&
 security_get_boolean_active("virt_use_nfs") != 1) {
@@ -1290,6 +1277,19 @@ virSecuritySELinuxSetFileconImpl(const char *path,
 VIR_INFO("Setting security context '%s' on '%s' not supported",
  tcon, path);
 }
+} else {
+/* However, don't claim error if SELinux is in Enforcing mode and
+ * we are running as unprivileged user and we really did see EPERM.
+ * Otherwise we want to return error if SELinux is Enforcing. */
+if (security_getenforce() == 1 &&
+(setfilecon_errno != EPERM || privileged)) {
+virReportSystemError(setfilecon_errno,
+ _("unable to set security context '%s' on 
'%s'"),
+ tcon, path);
+return -1;
+}
+VIR_WARN("unable to set security context '%s' on '%s' (errno %d)",
+ tcon, path, setfilecon_errno);
 }
 
 return 1;
-- 
2.39.1



[PATCH v2 2/2] selinux: Don't ignore ENOENT in Permissive mode

2023-02-17 Thread Michal Privoznik
In selinux driver there's virSecuritySELinuxSetFileconImpl()
which is responsible for actual setting of SELinux label on given
file and handling possible failures. In fhe failure handling code
we decide whether failure is fatal or not. But there is a bug:
depending on SELinux mode (Permissive vs. Enforcing) the ENOENT
is either ignored or considered fatal. This not correct - ENOENT
must always be fatal for couple of reasons:

- In virSecurityStackTransactionCommit() the seclabels are set
  for individual secdrivers (e.g. SELinux first and then DAC),
  but if one secdriver succeeds and another one fails, then no
  rollback is performed for the successful one leaking remembered
  labels.

- QEMU would fail opening the file anyways (if neither of
  secdrivers reported error and thus cancelled domain startup)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2004850
Signed-off-by: Michal Privoznik 
---
 src/security/security_selinux.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e9c4051a98..2e9efa78f4 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1280,9 +1280,11 @@ virSecuritySELinuxSetFileconImpl(const char *path,
 } else {
 /* However, don't claim error if SELinux is in Enforcing mode and
  * we are running as unprivileged user and we really did see EPERM.
- * Otherwise we want to return error if SELinux is Enforcing. */
-if (security_getenforce() == 1 &&
-(setfilecon_errno != EPERM || privileged)) {
+ * Otherwise we want to return error if SELinux is Enforcing, or we
+ * saw EPERM regardless of SELinux mode. */
+if (setfilecon_errno == ENOENT ||
+(security_getenforce() == 1 &&
+ (setfilecon_errno != EPERM || privileged))) {
 virReportSystemError(setfilecon_errno,
  _("unable to set security context '%s' on 
'%s'"),
  tcon, path);
-- 
2.39.1



[PATCH v2 0/2] selinux: Don't ignore ENOENT in Permissive mode

2023-02-17 Thread Michal Privoznik
This is just a resend of the following series:

https://listman.redhat.com/archives/libvir-list/2021-October/msg00738.html

Michal Prívozník (2):
  selinux: Swap two blocks handling setfilecon_raw() failure
  selinux: Don't ignore ENOENT in Permissive mode

 src/security/security_selinux.c | 32 +---
 1 file changed, 17 insertions(+), 15 deletions(-)

-- 
2.39.1



[PATCH 2/2] libvirt-nodedev: Allow read-only access to virNodeDeviceGetAutostart

2023-02-17 Thread Peter Krempa
Fetching whether a node-device is marked for autostart can be allowed
from read-only connections similarly to other objects.

Fixes: c6607a25b93
Signed-off-by: Peter Krempa 
---
 src/libvirt-nodedev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 1b7dee113e..366d2cfdbe 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -1089,7 +1089,6 @@ virNodeDeviceGetAutostart(virNodeDevicePtr dev,
 virResetLastError();

 virCheckNodeDeviceReturn(dev, -1);
-virCheckReadOnlyGoto(dev->conn->flags, error);

 if (dev->conn->nodeDeviceDriver &&
 dev->conn->nodeDeviceDriver->nodeDeviceGetAutostart) {
-- 
2.39.1



[PATCH 0/2] Read-only access to node devices

2023-02-17 Thread Peter Krempa
See individual patches.

Peter Krempa (2):
  access: Allow 'node-device.read' permission for anonymous users
  libvirt-nodedev: Allow read-only access to virNodeDeviceGetAutostart

 src/access/viraccessperm.h | 1 +
 src/libvirt-nodedev.c  | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

-- 
2.39.1



[PATCH 1/2] access: Allow 'node-device.read' permission for anonymous users

2023-02-17 Thread Peter Krempa
For all other objects we allow the 'read' permission for anonymous
users. In fact the idea is to allow all permissions users using the
readonly connection would have.

This impacts the following APIs (in terms of RPC procedure names):

  $ git grep -A 3 node_device:read | grep REMOTE
  src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_GET_XML_DESC = 114,
  src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_GET_PARENT = 115,
  src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_NUM_OF_CAPS = 116,
  src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_LIST_CAPS = 117,
  src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_GET_AUTOSTART = 433,
  src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_IS_PERSISTENT = 435,
  src/remote/remote_protocol.x-REMOTE_PROC_NODE_DEVICE_IS_ACTIVE = 436,

Fixes: a93cd08f
Signed-off-by: Peter Krempa 
---
 src/access/viraccessperm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
index 051246a7b6..2f04459ed9 100644
--- a/src/access/viraccessperm.h
+++ b/src/access/viraccessperm.h
@@ -473,6 +473,7 @@ typedef enum {
 /**
  * @desc: Read node device
  * @message: Reading node device configuration requires authorization
+ * @anonymous: 1
  */
 VIR_ACCESS_PERM_NODE_DEVICE_READ,

-- 
2.39.1



Re: [PATCH v2 0/5] qemu_passt: Fix issues with PID file

2023-02-17 Thread Michal Prívozník
On 2/16/23 17:35, Laine Stump wrote:
> On 2/16/23 8:32 AM, Michal Privoznik wrote:
>> This is a v2 of:
>>
>> https://listman.redhat.com/archives/libvir-list/2023-February/237731.html
>>
>> diff to v1:
>> - Merged patches that were ACKed in v1,
>> - Dropped 4/4 from the original series (the one that sets --foreground),
>>    and implemented a different approach
>>
>> Michal Prívozník (5):
>>    qemu_passt: Avoid double daemonizing passt
>>    qemu_passt: Report passt's error on failed start
>>    qemu_passt: Make passt report errors to stderr whenever possible
>>    qemu_passt: Deduplicate passt killing code
>>    qemu_passt: Let passt write the PID file
> 
> This is everything that was in the patch I sent last week, with the
> following additions
> 
> 1) adding NULLSTR() around the reference to errbuf in patch 2/5
> 
> 2) adding "--stderr" to the commandline in patch 3/5 (which I found to
> be unnecessary in my testing - as Stefano says everything goes to stderr
> until passt has completed its init anyway)
> 
> 3) the other bit of patch 3/5 which adds an extra message telling the
> user to look into the designated logfile for the error - this is
> unnecessary (and actually now counter-productive, as it forces you to
> look elsewhere for the error when you wouldn't have needed to) because
> of patches I've sent to passt.
> 
> 4)  patch 4/5 that is a cleanup de-duplicating code
> 
> 5) patch 5 changes additional code (that I didn't touch in my patch) to
> use virPidFileReadPath() instead of virPidFileReadPathIfLocked(), and
> virProcessKillPainfully() instead of the higher level
> virPidFileForceCleanupPath().
> 
> So it all seems fine (except the error reporting stuff), but why revert
> a patch only to push back the same changes in a deconstructed fashion
> plus some fixups, rather than just posting a followup or two?

Yeah, I realized this too and I'm sorry. My original intention was to
fix this in a completely different way (as my last patch from v1
demonstrates) and that was incompatible with yours.

Michal



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Stefan Weil

On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:


Which 32-bit hosts are still useful, and why?



Citing my previous mail:

   I now checked all downloads of the latests installers since 2022-12-30.

   qemu-w32-setup-20221230.exe – 509 different IP addresses
   qemu-w64-setup-20221230.exe - 5471 different IP addresses

   339 unique IP addresses are common for 32- and 64-bit, either
   crawlers or people who simply got both variants. So there remain 170
   IP addresses which only downloaded the 32-bit variant in the last week.

   I see 437 different strings for the browser type, but surprisingly
   none of them looks like a crawler.

So there still seems to be a certain small need for QEMU installers for 
32-bit Windows: 170 users für 32 bit only, 339 users for both 32 and 64 
bit, 5132 users for 64 bit only.


Stefan


[PATCH] qemu: fix reconnect of unix socket is wrong

2023-02-17 Thread Zhenguo Yao
'reconnect' parameter doesn't pass to qemu properly when
hotplug vhost-user device to vm. Fix this by making
'reconnect' to get correct value.

Signed-off-by: Zhenguo Yao 
---
 src/qemu/qemu_monitor_json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d05ca2932f..ba6276ec8e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6450,7 +6450,7 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
 return NULL;
 
 if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES)
-reconnect = chr->data.tcp.reconnect.timeout;
+reconnect = chr->data.nix.reconnect.timeout;
 else if (chr->data.nix.reconnect.enabled == 
VIR_TRISTATE_BOOL_NO)
 reconnect = 0;
 }
-- 
2.32.0



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Daniel P . Berrangé
On Fri, Feb 17, 2023 at 12:05:46PM +0100, Stefan Weil wrote:
> On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
> 
> > Which 32-bit hosts are still useful, and why?
> 
> 
> Citing my previous mail:
> 
>I now checked all downloads of the latests installers since 2022-12-30.
> 
>qemu-w32-setup-20221230.exe – 509 different IP addresses
>qemu-w64-setup-20221230.exe - 5471 different IP addresses
> 
>339 unique IP addresses are common for 32- and 64-bit, either
>crawlers or people who simply got both variants. So there remain 170
>IP addresses which only downloaded the 32-bit variant in the last week.
> 
>I see 437 different strings for the browser type, but surprisingly
>none of them looks like a crawler.
> 
> So there still seems to be a certain small need for QEMU installers for
> 32-bit Windows: 170 users für 32 bit only, 339 users for both 32 and 64 bit,
> 5132 users for 64 bit only.

The question which is hard/impossible to answer is whether the people
who downloaded the 32-bit build genuinely needed a 32-bit build or
just did so out of habit or confusion.

I know you can't believe everything you see with statistics, but as an
example, the chart at the bottom of this page suggests new deployments
of 32-bit Windows are negligible today:

https://www.pcbenchmarks.net/os-marketshare.html

there are existing deployments not accounted for there, but that may
still suggest many of the 32-bit downloads of QEMU will end up being
run on 64-bit hosts.

If we were to apply our support platform rule of only targetting the
latest 2 versions of the OS, this limits our targets to Win 10 and
Win 11. Windows 11 dropped 32-bit IIUC, so we're talking about
32-bit installs of Windows 10 only - even in Win10 days all new
physical hardware would have been 64-bit capable.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Markus Armbruster
Stefan Weil  writes:

> On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
>
>> Which 32-bit hosts are still useful, and why?
>
>
> Citing my previous mail:
>
>I now checked all downloads of the latests installers since 2022-12-30.
>
>qemu-w32-setup-20221230.exe – 509 different IP addresses
>qemu-w64-setup-20221230.exe - 5471 different IP addresses
>
>339 unique IP addresses are common for 32- and 64-bit, either
>crawlers or people who simply got both variants. So there remain 170
>IP addresses which only downloaded the 32-bit variant in the last week.
>
>I see 437 different strings for the browser type, but surprisingly
>none of them looks like a crawler.
>
> So there still seems to be a certain small need for QEMU installers for 
> 32-bit Windows: 170 users für 32 bit only, 339 users for both 32 and 64 bit, 
> 5132 users for 64 bit only.

Actual data is always welcome!

I wonder, though...  given how decrepit 32-bit-only PCs must now be, and
how well Windows 10 (the only version still supporting them) runs on
these, how many downloaders of the w32 version could (and quite probably
should) use the w64 version instead?



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Claudio Fontana
On 1/30/23 12:44, Thomas Huth wrote:
> Testing 32-bit host OS support takes a lot of precious time during the QEMU
> contiguous integration tests, and considering that many OS vendors stopped
> shipping 32-bit variants of their OS distributions and most hardware from
> the past >10 years is capable of 64-bit, keeping the 32-bit support alive
> is an inadequate burden for the QEMU project. Let's mark the 32-bit
> support as deprecated so we can drop it after a while - this will help
> us to cut down our limited CI minutes in the gitlab CI, for example.
> 
> Signed-off-by: Thomas Huth 
> ---
>  docs/about/deprecated.rst | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 9f1bbc495d..ce6463e72b 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -181,9 +181,20 @@ As Debian 10 ("Buster") moved into LTS the big endian 32 
> bit version of
>  MIPS moved out of support making it hard to maintain our
>  cross-compilation CI tests of the architecture. As we no longer have
>  CI coverage support may bitrot away before the deprecation process
> -completes. The little endian variants of MIPS (both 32 and 64 bit) are
> +completes. The little endian variants of MIPS are
>  still a supported host architecture.
>  
> +32-bit host operating systems (since 8.0)
> +'
> +
> +Testing 32-bit host OS support takes a lot of precious time during the QEMU
> +contiguous integration tests, and considering that many OS vendors stopped
> +shipping 32-bit variants of their OS distributions and most hardware from
> +the past >10 years is capable of 64-bit, keeping the 32-bit support alive
> +is an inadequate burden for the QEMU project. Thus QEMU will soon drop the
> +support for 32-bit host systems.
> +
> +
>  QEMU API (QAPI) events
>  --
>  

>From our (SUSE) support perspective, we do not support 32bit virtualization 
>hosts downstream,

and in general I am in favor of the change.

Ciao,

Claudio








Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Daniel P . Berrangé
On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
> I feel the discussion petered out without a conclusion.
> 
> I don't think letting the status quo win by inertia is a good outcome
> here.
> 
> Which 32-bit hosts are still useful, and why?

Which 32-bit hosts does Linux still provide KVM  support for.

If any, is there an EOL date for Linux 32-bit KVM support ?

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Claudio Fontana
On 2/17/23 11:36, Markus Armbruster wrote:
> I feel the discussion petered out without a conclusion.
> 
> I don't think letting the status quo win by inertia is a good outcome
> here.
> 
> Which 32-bit hosts are still useful, and why?

Hi Markus,
 
if the question is very very general, my opinion is that some of the most 
useful 32-bit systems are things like ARMv8-R capable systems for deeply 
embedded real time applications.

Ciao,

Claudio




Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Daniel P . Berrangé
On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
> I feel the discussion petered out without a conclusion.
> 
> I don't think letting the status quo win by inertia is a good outcome
> here.
> 
> Which 32-bit hosts are still useful, and why?
> 
> Please note my question is not about the cost of keeping them (or
> savings from not keeping them), it's about the value they provide.  When
> value rounds to zero, cost is irrelevant, so let's get a firm idea of
> value *first*.

With my OS maintainer hat on, 32-bit host support is not relevant
to Fedora[1] or RHEL. Both have dropped all 32-bit host install
support, only maintaining 32-bit builds of some libaries for use
on 64-bit host install, which is not relevant to QEMU's deliverables.

With regards,
Daniel

[1] Arm7 was the last 32-bit host in Fedora, dropped after F36
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-17 Thread Markus Armbruster
I feel the discussion petered out without a conclusion.

I don't think letting the status quo win by inertia is a good outcome
here.

Which 32-bit hosts are still useful, and why?

Please note my question is not about the cost of keeping them (or
savings from not keeping them), it's about the value they provide.  When
value rounds to zero, cost is irrelevant, so let's get a firm idea of
value *first*.



Re: [libvirt PATCH v5 31/32] schema: add keyfile configuration for ssh disks

2023-02-17 Thread Peter Krempa
On Tue, Feb 14, 2023 at 11:08:18 -0600, Jonathon Jongsma wrote:
> Authenticating via key file to an ssh server is often preferable to
> logging in via password. In order to support this functionality add a
> new  xml element for ssh disks that allows the user to specify
> a keyfile, username and optional ssh-agent socket location. Example
> configuration:
> 
> 
>   
> 
> ...
>   
> ...
> 
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  docs/formatdomain.rst |  8 
>  src/conf/schemas/domaincommon.rng | 22 +-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index d5ad5d80b0..ea3d1a5a06 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2945,6 +2945,14 @@ paravirtualized driver is specified via the ``disk`` 
> element.
>of these attributes is omitted, then that field is assumed to be the
>default value for the current system. If both ``user`` and ``group``
>are intended to be default, then the entire element may be omitted.
> +
> +  When using an ``ssh`` protocol, this element is used to enable
> +  authentication via ssh keys. In this configuration, the element has 
> three
> +  attributes. The ``username`` attribute specifies the name of the user 
> on
> +  the remote server. A path to an ssh key can be specified in the
> +  ``keyfile`` attribute. If the ssh key is password-protected, the key 
> can
> +  be added to an ssh-agent and the path to the ssh-agent socket can be
> +  specified in the ``agentsock`` attribute.

Reword this so that it says that the ssh key can be used with an agent
even when it is not protected. Or in fact promote the agent first and
mention that a password-less key can be used without an agent with the
keyfile option.

> ``reconnect``
>For disk type ``vhostuser`` configures reconnect timeout if the 
> connection
>is lost. It has two mandatory attributes:
> diff --git a/src/conf/schemas/domaincommon.rng 
> b/src/conf/schemas/domaincommon.rng
> index f38f1f3ff1..a15ce97ef3 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -2168,6 +2168,22 @@
>  
>
>  
> +  
> +
> +  
> +
> +  
> +  
> +
> +  
> +  
> +
> +  
> +
> +  

And tweak the schema to allow agent without keyfile.

Reviewed-by: Peter Krempa 



Re: [libvirt PATCH v5 29/32] schema: add configuration for host verification of ssh disks

2023-02-17 Thread Peter Krempa
On Thu, Feb 16, 2023 at 16:59:33 -0600, Jonathon Jongsma wrote:
> On 2/16/23 10:45 AM, Peter Krempa wrote:
> > On Tue, Feb 14, 2023 at 11:08:16 -0600, Jonathon Jongsma wrote:
> > > In order to make ssh disks usable, we need to be able to validate a
> > > remote host. To do this, add a  xml element for ssh disks to
> > > allow the user to specify a location for a file that contains known host
> > > keys. Implementation to follow.
> > > 
> > > Signed-off-by: Jonathon Jongsma 
> > > ---
> > >   docs/formatdomain.rst |  6 ++
> > >   src/conf/schemas/domaincommon.rng | 11 +++
> > >   2 files changed, 17 insertions(+)
> > > 
> > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > > index bf071255c5..d5ad5d80b0 100644
> > > --- a/docs/formatdomain.rst
> > > +++ b/docs/formatdomain.rst
> > > @@ -2953,6 +2953,12 @@ paravirtualized driver is specified via the 
> > > ``disk`` element.
> > >If the reconnect feature is enabled, accepts ``yes`` and ``no``
> > > ``timeout``
> > >The amount of seconds after which hypervisor tries to 
> > > reconnect.
> > > +   ``knownHosts``
> > > +  For storage accessed via the ``ssh`` protocol, this element 
> > > configures a
> > > +  path to a file containing a list of known ssh hosts to be used to 
> > > verify
> > > +  the remote host. The location of the file is specified via the 
> > > ``path``
> > > +  attribute.
> > > +  :since:`Since 9.1.0`
> > 
> > How does nbdkit do enrollment here? Does it expect a pre-filled set of
> > known hosts? Or does it allow new host on first use?
> > 
> 
> It expects a prefilled known hosts file. Here's what it says in the manpage
> for nbdkit-ssh-plugin:
> 
>   Known hosts
>The SSH server’s host key is checked at connection time, and must be
> present and correct in the local "known hosts" file.
> 
>If you have never connected to the SSH server before then the
> connection will usually fail.  You can:
> 
>•   connect to the server first using ssh(1) so you can manually
> accept the host key, or
> 
>•   provide the host key in an alternate file which you specify using
> the "known-hosts" option, or
> 
>•   set verify-remote-host=false on the command line.  This latter
> option is dangerous because it allows a MITM attack to be conducted against
> you.

Okay. The fact that it expects a pre-filled knownHosts should be
mentioned in the docs. I think it's a reasonable limitation. I'd not
bother with allowing to disable verification at all.

With docs updated:

Reviewed-by: Peter Krempa 



Re: [libvirt PATCH v5 27/32] schema: add password configuration for ssh disk

2023-02-17 Thread Peter Krempa
On Thu, Feb 16, 2023 at 16:51:46 -0600, Jonathon Jongsma wrote:
> On 2/16/23 10:43 AM, Peter Krempa wrote:
> > On Tue, Feb 14, 2023 at 11:08:14 -0600, Jonathon Jongsma wrote:
> > > Right now, ssh network disks are not usable. There is some basic support
> > > in libvirt that is meant to support disk chains that have backing disks
> > > located at ssh urls, but there is no real way for a user to configure a
> > > ssh-based disk.  This commit allows users to configure an ssh disk with
> > > password authentication. Implementation will follow.
> > > 
> > > 
> > >
> > >  
> > >
> > >  
> > > 
> > > 
> > > Signed-off-by: Jonathon Jongsma 
> > > ---
> > >   docs/formatdomain.rst | 27 ++-
> > >   src/conf/schemas/domaincommon.rng | 23 ++-
> > >   2 files changed, 36 insertions(+), 14 deletions(-)

[...]

> > > +  the password). Known secret types are "ceph" for Ceph RBD network 
> > > sources
> > > +  and "iscsi" for CHAP authentication of iSCSI targets. Both will 
> > > require
> > > +  either a ``uuid`` attribute with the UUID of the secret object or a
> > > +  ``usage`` attribute matching the key that was specified in the 
> > > secret
> > > +  object.
> > 
> > This paragraph doesn't really state what to put into 'type' for ssh as
> > 'ceph' and 'iscsi' are only mentioned. For 'ssh' we need a 'ssh' type.
> 
> Hmm, do we also need a separate type for http auth as well, then? At the
> moment we seem to just re-use the 'iscsi' type for all of the http auth in
> our tests (e.g. disk-cdrom-network.xml, etc).

Good point! Let's deal with that later.

Reviewed-by: Peter Krempa