Re: [libvirt] [PATCH] support auth for qemu SCSI hotplug

[John Ferlan]

On 10/15/2016 10:04 AM, Gema Gomez wrote:
> Hi John,
> 
> On 13/10/16 21:37, John Ferlan wrote:
>> So could you provide a bit more information about the configuration.
>> Are you indicating that you have an RBD pool with a volume that's being
>> used as a SCSI device on the guest?
> 
> We are indeed using Ceph (RBD) pool volumes, attached via virtio-scsi to
> the guests.
> 
>> Reason I ask - not modifying qemuDomainAttachSCSIDisk was by choice
>> mainly because it's generally used with the iSCSI pool which at this
>> point in time cannot support this new secret model.
> 
> Even though iSCSI doesn't support secrets this way, doesn't mean it
> isn't necessary for RBD. In particular, the current handling is
> inconsistent between domain creation and hotplugging of a volume. On
> domain creation, the secret object is added just fine.
> 
> On hotplug, when libvirt talks to the qemu monitor, it tells qemu to
> create a virtio-scsi device, rbd-backed, with the secret pointing to a
> secret object. However, that secret object is *NOT* currently being
> inserted via the qemu mon communication, and so the command fails to
> actually attach the disk.
> 
> Considering libvirt is already telling qemu on hotplug that there is
> some secret with a given name, it sounds logical to actually add that
> secret object. Plus, that's consistent, as I said, with how domain
> creation works.
> 
> As for iSCSI not supporting it - I'm not sure I see the problem. The
> patch I submitted qualifies the creation of the aes key object with
> whether secinfo is present for the disk, and it's of AES type.
> 
> And for reference, below is the conversation libvirt and the qemu
> monitor were having before this patch, including the XML. Since libvirt
> wasn't adding the scsi0-0-0-1-secret0 object, it all failed rather
> miserably.
> 
> 2016-10-07 14:09:40.974+: 13608: info : qemuMonitorIOWrite:534 :
> QEMU_MONITOR_IO_WRITE: mon=0x7f7c00eb60
> buf={"execute":"human-monitor-command","arguments":{"command-line":"drive_add
> 
> dummy
> file=rbd:volumes/volume-e51d02fc-7399-4e51-bdde-84577ba79990:id=nova:auth_supported=cephx\\;none:mon_host=10.10.0.101\\:6789\\;10.10.0.111\\:6789\\;10.10.0.112\\:6789,file.password-secret=scsi0-0-0-1-secret0,format=raw,if=none,id=drive-scsi0-0-0-1,serial=e51d02fc-7399-4e51-bdde-84577ba79990,cache=none"},"id":"libvirt-14"}
> 
> 
> 2016-10-07 14:09:40.987+: 13608: info : qemuMonitorIOProcess:429 :
> QEMU_MONITOR_IO_PROCESS: mon=0x7f7c00eb60 buf={"return": "No secret with
> id 'scsi0-0-0-1-secret0'\r\n", "id": "libvirt-14"}
>  len=79
> 
> for this XML:
> 
> 
>   
>name="volumes/volume-e51d02fc-7399-4e51-bdde-84577ba79990">
> 
> 
> 
>   
>   
> 
>   
>   
>   e51d02fc-7399-4e51-bdde-84577ba79990
> 
> 
> Thanks,
> Gema
> 

OK thanks for confirming my suspicion...

I'd like to add/merge the attached to this patch.  Essentially it's a
test that uses XML like above. Although it's added to the qemu_command
processing - it shows the need to have the SCSI hotplug code to also
have the secret processing. I still haven't figured out those hotplug
tests - if you want to take a shot, be my guest!

Just let me know and I'll merge it with yours and push.

Thanks and congrats on your first libvirt patch!

John
>From b5acf85351360bedb1ddb0e66c90f0dcc730cdd2 Mon Sep 17 00:00:00 2001
From: John Ferlan 
Date: Wed, 19 Oct 2016 18:54:05 -0400
Subject: [PATCH] tests: Merge test for RBD SCSI hotplug

NB: The SCSI hot unplug code will use the qemuDomainDetachDiskDevice
which calls qemuDomainRemoveDiskDevice which will make an attempt to
remove the secret object.

Signed-off-by: John Ferlan 
---
 .../qemuxml2argv-disk-drive-network-rbd-auth-AES.args  | 14 --
 .../qemuxml2argv-disk-drive-network-rbd-auth-AES.xml   | 13 +
 tests/qemuxml2argvtest.c   |  2 +-
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args
index 07d01b6..d536136 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args
@@ -18,6 +18,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
 -no-acpi \
 -boot c \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
 -usb \
 -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
@@ -28,5 +29,14 @@ keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
 mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:\
 6322,file.password-secret=virtio-disk0-secret0,format=raw,if=none,\
 id=drive-virtio-disk0' \
--device 

Re: [libvirt] [RFC] Toward a better NEWS file

[John Ferlan]

On 10/19/2016 07:53 AM, Andrea Bolognani wrote:
> Hi,
> 
> there's an idea that has been kicking around in my head for
> a while, and I'd like to share it with the list to gather
> some feedback before I forget about it :)
> 
> Right now, each entry in our NEWS file contains what is
> basically the output of
> 
>   git log \
>   --pretty=oneline \
>   vX.Y-1.0..vX.Y.0
> 
> with the commits organized somewhat arbitrarily into a bunch
> of sections with partially overlapping scopes.
> 
> I believe the current form is less than useful, as it is too
> detailed for end users and distro packagers, who only care
> about the high-level user visible changes, and not detailed
> enough for developers, who are always going to refer to the
> proper git log anyway. Moreover, we ship a ChangeLog file
> that contains very detailed information already.
> 
> Ideally, the NEWS file would contain a summary of notable
> changes (new APIs, significantly improved features, etc.)
> laid out in an easy to digest fashion, such that people who
> are not knee-deep into libvirt development can grasp them
> and hopefully get excited about upgrading to our latest and
> greatest release :)
> 
> Of course, it would take an insane amount of time for any
> single one of us to turn the git log into such a document,
> and the result would still be sub-par because we simply
> can't expect anyone to have full insight in every single
> change to the project.
> 
> My solution for this is to ask the people with the most
> knowledge of the changes - the authors themselves!
> 
> The workflow I'm envisioning would look like this:
> 
>   * DV, at the same time as he announces that libvirt has
> entered freeze, will put out a Call for NEWS and ask
> people who have contributed code to the upcoming release
> to post a summary of their changes

And hope (the quick list that comes to mind):

 1. They are paying attention
 2. Decide to do so in a timely manner
 3. They're not on vacation
 4. They're not unwilling to do it
 5. They're not "too busy" doing something else "more important"

> 
>   * the authors will go over
> 
>   git log \
>   --author=$(git config user.email) \
>   vX.Y-1.0..master
> 
> and come up with a short (one-three sentences) summary
> for each of the changes, if they are notable. Commits
> that are part of a larger series would not be described
> on their own: a short summary of the series would be
> used instead, akin to the one you would put in your
> cover letter.

There are those of us who will have a hard time with "short" and
"one-three sentences" while there are others that would summarize their
changes as "" or "fixed some bugs".  Just go through recent history and
look at the cover letters and commit messages for examples.

> 
> To give a practical example: I've mostly been busy with
> reviews this cycle, but if I were to go over my commits
> since v2.3.0 right now I would write something like
> 
>   * Bug fix: don't restart libvirt-guests.service when
> libvirtd.service is restarted
> 
> for commit 2273bbd, and omit both 61e1014 and a0da413 as
> they're neither notable enough on their own, nor part of
> a larger series: we'll always have a "various bug fixes
> and improvements" bullet point in a NEWS file entry to
> take care of that kind of small cleanups and improvements.
> 
>   * the authors would post the resulting summaries to the
> list. We could simply post them as regular patches to
> docs/news.html.in (potentially without requiring review
> before pushing them), or post them as plain text and have
> someone collect them and prepare a single commit

Not review potential speling and grammer issues ;-) that someone will
come along and want to fix some day? Would the fix be newsworthy?
(facetiously said of course just in case).

> 
>   * DV will tag the release and push the tarballs, and
> everyone will be able to enjoy the NEWS file :)
> 
> Some light editoral work might be needed throughout the
> process, eg. fix typos or post one or two reminders during
> the freeze: I volunteer to take such tasks upon myself.
> 
> I'm looking forward to feedback about this idea, especially
> from people who might be part of any community where anything
> like this is already happening.
> 

I really think we should do something - especially to be able to
describe what's new, what was added when, etc. beyond what DV culls out
of the existing commits.

Whether the mechanism is some news.html.in or features.html.in or
something else is fine. I don't think we wait until the end of the
release to update. Manually going through patches and matching up to
cover letters by one person for some releases will be easy, while for
other releases it will be an arduous task. That becomes a bottleneck on
one person and could be a time consuming task.

I was thinking after KVM Forum it would be nice if we had some way to

[libvirt] [PATCH 2/3] Use new util function to check network name

[Sławek Kapłoński]

New util function virXMLNodeHasIllegalChars is now used to test if
parsed network contains illegal char '/' in it's name.
---
 src/conf/network_conf.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index aa39776..bd934f1 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2117,11 +2117,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
 goto error;
 }
 
-if (strchr(def->name, '/')) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("name %s cannot contain '/'"), def->name);
+if (virXMLNodeHasIllegalChars("name", def->name, "/"))
 goto error;
-}
 
 /* Extract network uuid */
 tmp = virXPathString("string(./uuid[1])", ctxt);
-- 
2.10.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] util: Add function to check if string contains some illegal chars

[Sławek Kapłoński]

This new function can be used to check if e.g. name of XML
node don't contains forbidden chars like "/" or "\n".
---
 src/libvirt_private.syms |  1 +
 src/util/virxml.c| 28 
 src/util/virxml.h|  3 +++
 3 files changed, 32 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 55b6a24..f91f179 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2678,6 +2678,7 @@ virUUIDParse;
 # util/virxml.h
 virXMLChildElementCount;
 virXMLExtractNamespaceXML;
+virXMLNodeHasIllegalChars;
 virXMLNodeSanitizeNamespaces;
 virXMLNodeToString;
 virXMLParseHelper;
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 03bd784..b0d0a94 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -462,6 +462,34 @@ virXPathLongLong(const char *xpath,
 return ret;
 }
 
+
+/**
+ * virXMLNodeHasIllegalChars: checks if string contains at least one of
+ * forbidden characters
+ *
+ * @node_name: Name of checked node
+ * @node: string to check
+ * @chars: illegal chars to check
+ *
+ * If node contains any of illegal chars VIR_ERR_XML_DETAIL error will be
+ * reported.
+ *
+ * Returns: 0 if string don't contains any of given characters, -1 otherwise
+ */
+int
+virXMLNodeHasIllegalChars(const char *node_name,
+  const char *node,
+  const char *chars)
+{
+if (strpbrk(node, chars)) {
+virReportError(VIR_ERR_XML_DETAIL,
+   _("invalid char in %s: 0x%02x"), node_name, *chars);
+return -1;
+}
+return 0;
+}
+
+
 /**
  * virXMLPropString:
  * @node: XML dom node pointer
diff --git a/src/util/virxml.h b/src/util/virxml.h
index 7a0a1da..4425cf8 100644
--- a/src/util/virxml.h
+++ b/src/util/virxml.h
@@ -71,6 +71,9 @@ xmlNodePtr  virXPathNode(const char *xpath,
 int  virXPathNodeSet(const char *xpath,
  xmlXPathContextPtr ctxt,
  xmlNodePtr **list);
+intvirXMLNodeHasIllegalChars(const char *node_name,
+ const char *node,
+ const char *chars);
 char *  virXMLPropString(xmlNodePtr node,
  const char *name);
 long virXMLChildElementCount(xmlNodePtr node);
-- 
2.10.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/3] Forbid new-line char in name of new networks

[Sławek Kapłoński]

New line character in name of network is now forbidden because it
mess virsh output and can be confusing for users.
Validation of name is done in network driver, after parsing XML to avoid
problems with dissappeared network which was already created with
new-line char in name.

Closes-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=818064
---
 src/network/bridge_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b2af482..a5a116a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2973,6 +2973,9 @@ networkValidate(virNetworkDriverStatePtr driver,
 bool bandwidthAllowed = true;
 bool usesInterface = false, usesAddress = false;
 
+if (virXMLNodeHasIllegalChars("name", def->name, "\n"))
+return -1;
+
 /* Only the three L3 network types that are configured by libvirt
  * need to have a bridge device name / mac address provided
  */
-- 
2.10.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v4 0/3] Forbid new-line char in name of networks

[Sławek Kapłoński]

v3: http://www.redhat.com/archives/libvir-list/2016-October/msg00627.html

Differences in v4:
* function to check string moved from src/util/virstring to src/util/virxml

Sławek Kapłoński (3):
  util: Add function to check if string contains some illegal chars
  Use new util function to check network name
  Forbid new-line char in name of new networks

 src/conf/network_conf.c |  5 +
 src/libvirt_private.syms|  1 +
 src/network/bridge_driver.c |  3 +++
 src/util/virxml.c   | 28 
 src/util/virxml.h   |  3 +++
 5 files changed, 36 insertions(+), 4 deletions(-)

-- 
2.10.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v10 4/4] qemu: Add secret object hotplug for TCP chardev TLS

[John Ferlan]

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

Complete the implementation of support for TLS encryption on
chardev TCP transports by adding the hotplug ability of a secret
to generate the passwordid for the TLS object

Likewise, add the ability to hot unplug that secret object as well

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c  |  2 +-
 src/qemu/qemu_hotplug.c | 57 -
 src/qemu/qemu_hotplug.h |  3 ++-
 tests/qemuhotplugtest.c |  2 +-
 4 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8789c9d..5a1cf7b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7567,7 +7567,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 break;
 
 case VIR_DOMAIN_DEVICE_CHR:
-ret = qemuDomainAttachChrDevice(driver, vm,
+ret = qemuDomainAttachChrDevice(conn, driver, vm,
 dev->data.chr);
 if (!ret) {
 alias = dev->data.chr->info.alias;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 59c3b25..4935bbf 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1690,7 +1690,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
 return ret;
 }
 
-int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
+int qemuDomainAttachChrDevice(virConnectPtr conn,
+  virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainChrDefPtr chr)
 {
@@ -1704,8 +1705,11 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 char *charAlias = NULL;
 bool chardevAttached = false;
 bool tlsobjAdded = false;
+bool secobjAdded = false;
 virJSONValuePtr tlsProps = NULL;
 char *tlsAlias = NULL;
+virJSONValuePtr secProps = NULL;
+char *secAlias = NULL;
 bool need_release = false;
 
 if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
@@ -1729,12 +1733,28 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 if (qemuDomainChrPreInsert(vmdef, chr) < 0)
 goto cleanup;
 
+if (qemuDomainSecretChardevPrepare(conn, driver, priv, chr) < 0)
+goto cleanup;
+
 if (dev->type == VIR_DOMAIN_CHR_TYPE_TCP &&
 qemuDomainSupportTLSChardevTCP(cfg, dev)) {
+qemuDomainChardevPrivatePtr chardevPriv =
+QEMU_DOMAIN_CHARDEV_PRIVATE(chr);
+
+/* Add a secret object in order to access the TLS environment.
+ * The secinfo will only be created for serial TCP device. */
+if (chardevPriv && chardevPriv->secinfo) {
+if (qemuBuildSecretInfoProps(chardevPriv->secinfo, ) < 0)
+goto cleanup;
+
+if (!(secAlias = qemuDomainGetSecretAESAlias(charAlias, false)))
+goto cleanup;
+}
+
 if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
  dev->data.tcp.listen,
  cfg->chardevTLSx509verify,
- NULL,
+ secAlias,
  priv->qemuCaps,
  ) < 0)
 goto cleanup;
@@ -1745,6 +1765,15 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 }
 
 qemuDomainObjEnterMonitor(driver, vm);
+if (secAlias) {
+rc = qemuMonitorAddObject(priv->mon, "secret",
+  secAlias, secProps);
+secProps = NULL;
+if (rc < 0)
+goto exit_monitor;
+secobjAdded = true;
+}
+
 if (tlsAlias) {
 rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
   tlsAlias, tlsProps);
@@ -1775,6 +1804,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 qemuDomainReleaseDeviceAddress(vm, >info, NULL);
 VIR_FREE(tlsAlias);
 virJSONValueFree(tlsProps);
+VIR_FREE(secAlias);
+virJSONValueFree(secProps);
 VIR_FREE(charAlias);
 VIR_FREE(devstr);
 virObjectUnref(cfg);
@@ -1782,6 +1813,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 
  exit_monitor:
 orig_err = virSaveLastError();
+if (secobjAdded)
+ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
 if (tlsobjAdded)
 ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
 /* detach associated chardev on error */
@@ -4387,6 +4420,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
 virDomainDefPtr vmdef = vm->def;
 virDomainChrDefPtr tmpChr;
 char *objAlias = NULL;
+char *secAlias = NULL;
 char *devstr = NULL;
 char *charAlias = NULL;
 
@@ -4405,9 +4439,18 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
 goto cleanup;
 
 if (tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
-

[libvirt] [PATCH v10 0/4] Add native TLS encrypted chardev TCP support

[John Ferlan]

v9: http://www.redhat.com/archives/libvir-list/2016-October/msg00726.html

"Theorically speaking" patch #2 is "separate" from patches 1, 3, & 4. That
is patch 3 and 4 are adding the secret uuid processing handling which is
different than the enable/disable property logic for patch 2.  I've left
them all together though since just to be consistent with previous series.

Differences in v10

... Pushed the previous series 2/5 and 3/5 since they were ACK'd

... Create a new patch 1 to have helper qemuDomainSupportTLSChardevTCP
It's mostly unnecessary without patch 2 though, but it made adding
or "separating" patch 2 from patches 3 & 4 a whole lot easier...

... Modified former patch 1 (now patch 2) to accommodate for a paradigm
where tls='yes' and chardev_tls=0 might be possible. The new helper
is used to whether to add the TLS information or not.


... Modified former patch 4 (now patch 3) to accommodate for the
changes Pavel has made to the code and to generate the secalias
using the "charAlias"

... Modified former patch 4 (now patch 4) to use the "charAlias" as well
and merge in Pavel's changes

NOTE: Even though 'yes' is a now possibility, it is an option that's assuming
  chardev_tls=0 so I don't feel the issues raised during review of v8
  regarding needing to consider a currently running 2.3.0 domain that
  still needs to work when 2.4.0 is applied. I believe it will be with
  the way the optional property is being used, thus with respect to
  the points in:

http://www.redhat.com/archives/libvir-list/2016-October/msg00732.html

The proposed qemuProcessPrepareDomain change is invalid since haveTLS
is a tristate and chardevTLS is a bistate. This is what I meant about being
a bit dangerous (e.g. BOOL_NO=2, BOOL_YES=1, and BOOL_ABSENT=0); however,
"chardevTLS=1" is enabled (yes) and "chardevTLS=0" is disabled (absent).
While it looks good when typing, when you get down to the details sometimes
you find those 'gotchas'. Even if the shorthand logic were fixed, it's
not going to be good to assume that setting the domain property or
disabling the domain property is the desired action.

The qemuProcessAttach is for qemu-attach and not the path that libvirt
uses to reconnect to running domains (which is qemuProcessReconnect).
There's so much broken from the qemu-attach right now - I doubt it
really works at all.

With respect to the reconnect processing (since that's really what you
were thinking about)...  There is no "options" provided/found in that code.
New code could possibly "read" the '/proc/$pid/cmdline' file and look for
'tls-creds', but the only purpose of that would be to manage 'assumptions'
with how the "tls='{yes|no}'" property is used.

Altering virDomainChrSourceDefParseXML and virDomainChrSourceDefFormat
to manage some new boolean 'tlsFromConfig' that I'm not sure could be
set properly is something I think is outside these patches.

John Ferlan (4):
  qemu: Introduce qemuDomainSupportTLSChardevTCP
  domain: Add optional 'tls' attribute for TCP chardev
  qemu: Add a secret object to/for a chardev tcp with secret
  qemu: Add secret object hotplug for TCP chardev TLS

 docs/formatdomain.html.in  |  28 +
 docs/schemas/domaincommon.rng  |   5 +
 src/conf/domain_conf.c |  22 +++-
 src/conf/domain_conf.h |   1 +
 src/qemu/qemu_command.c|  33 -
 src/qemu/qemu_command.h|   1 +
 src/qemu/qemu_domain.c | 133 -
 src/qemu/qemu_domain.h |  18 ++-
 src/qemu/qemu_driver.c |   2 +-
 src/qemu/qemu_hotplug.c|  58 -
 src/qemu/qemu_hotplug.h|   3 +-
 src/qemu/qemu_process.c|   4 +-
 tests/qemuhotplugtest.c|   2 +-
 ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args |  30 +
 ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml |  50 
 ...xml2argv-serial-tcp-tlsx509-secret-chardev.args |  38 ++
 ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml |  50 
 tests/qemuxml2argvtest.c   |  20 
 ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |   1 +
 tests/qemuxml2xmltest.c|   1 +
 20 files changed, 483 insertions(+), 17 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml

-- 
2.7.4

[libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

[John Ferlan]

Add an optional "tls='yes|no'" attribute for a TCP chardev.

For QEMU, this will allow for disabling the host config setting of the
'chardev_tls' for a domain chardev channel by setting the value to "no" or
to attempt to use a host TLS environment when setting the value to "yes"
when the host config 'chardev_tls' setting is disabled, but a TLS environment
is configured via either the host config 'chardev_tls_x509_cert_dir' or
'default_tls_x509_cert_dir'

Alter qemuDomainSupportTLSChardevTCP to augment the decision points for
choosing whether to try to use TLS.

Signed-off-by: John Ferlan 
---
 docs/formatdomain.html.in  | 28 
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/domain_conf.c | 22 +-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_command.c|  2 +-
 src/qemu/qemu_domain.c | 20 +++--
 src/qemu/qemu_domain.h |  3 +-
 src/qemu/qemu_hotplug.c|  4 +-
 ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
 ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++
 tests/qemuxml2argvtest.c   |  3 ++
 ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
 tests/qemuxml2xmltest.c|  1 +
 13 files changed, 162 insertions(+), 8 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9051178..da6be67 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6204,6 +6204,34 @@ qemu-kvm -net nic,model=? /dev/null
   /devices
   ...
 
+
+  Since 2.4.0, the optional attribute
+  tls can be used to control whether a serial chardev
+  TCP communication channel would utilize a hypervisor configured
+  TLS X.509 certificate environment in order to encrypt the data
+  channel. For the QEMU hypervisor, usage of a TLS envronment can
+  be controlled on the host by the chardev_tls and
+  chardev_tls_x509_cert_dir or
+  default_tls_x509_cert_dir settings in the file
+  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
+  then unless the tls attribute is set to "no", libvirt
+  will use the host configured TLS environment.
+  If chardev_tls is disabled, but the tls
+  attribute is set to "yes", then libvirt will attempt to use the
+  host TLS environment if either the chardev_tls_x509_cert_dir
+  or default_tls_x509_cert_dir TLS directory structure exists.
+
+
+  ...
+  devices
+serial type="tcp"
+  source mode='connect' host="127.0.0.1" service="" tls="yes"/
+  protocol type="raw"/
+  target port="0"/
+/serial
+  /devices
+  ...
+
 UDP network console
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3106510..e6741bb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3453,6 +3453,11 @@
 
   
 
+
+  
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 89473db..e4fa9ad 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1997,6 +1997,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
 
 if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0)
 return -1;
+
+dest->data.tcp.haveTLS = src->data.tcp.haveTLS;
 break;
 
 case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -10040,6 +10042,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 char *master = NULL;
 char *slave = NULL;
 char *append = NULL;
+char *haveTLS = NULL;
 int remaining = 0;
 
 while (cur != NULL) {
@@ -10047,6 +10050,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 if (xmlStrEqual(cur->name, BAD_CAST "source")) {
 if (!mode)
 mode = virXMLPropString(cur, "mode");
+if (!haveTLS)
+haveTLS = virXMLPropString(cur, "tls");
 
 switch ((virDomainChrType) def->type) {
 case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -10223,6 +10228,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 def->data.tcp.listen = true;
 }
 
+if (haveTLS &&
+(def->data.tcp.haveTLS =
+ virTristateBoolTypeFromString(haveTLS)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("unknown chardev 'tls' setting '%s'"),
+ 

[libvirt] [PATCH v10 1/4] qemu: Introduce qemuDomainSupportTLSChardevTCP

[John Ferlan]

It's very simple right now, but it's about to get a bit more complicated.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_command.c |  2 +-
 src/qemu/qemu_domain.c  | 16 
 src/qemu/qemu_domain.h  |  1 +
 src/qemu/qemu_hotplug.c |  4 ++--
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8282162..d45a7de 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4943,7 +4943,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 if (dev->data.tcp.listen)
 virBufferAdd(, nowait ? ",server,nowait" : ",server", -1);
 
-if (cfg->chardevTLS) {
+if (qemuDomainSupportTLSChardevTCP(cfg)) {
 char *objalias = NULL;
 
 if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4c118ff..746d94f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6298,3 +6298,19 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
 
 return true;
 }
+
+
+/* qemuDomainSupportTLSChardevTCP
+ * @cfg: Pointer to driver cfg
+ *
+ * Let's check if this host supports using the TLS environment for chardev.
+ *
+ * Returns true if we want to use TLS, false otherwise.
+ */
+bool
+qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg)
+{
+if (cfg->chardevTLS)
+return true;
+return false;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 707a995..7ecafac 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -755,4 +755,5 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver,
 bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
 virQEMUCapsPtr qemuCaps);
 
+bool qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg);
 #endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2cb2267..c2b43b1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1730,7 +1730,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 goto cleanup;
 
 if (dev->type == VIR_DOMAIN_CHR_TYPE_TCP &&
-cfg->chardevTLS) {
+qemuDomainSupportTLSChardevTCP(cfg)) {
 if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
  dev->data.tcp.listen,
  cfg->chardevTLSx509verify,
@@ -4404,7 +4404,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
 goto cleanup;
 
 if (tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
-cfg->chardevTLS &&
+qemuDomainSupportTLSChardevTCP(cfg) &&
 !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
 goto cleanup;
 
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v10 3/4] qemu: Add a secret object to/for a chardev tcp with secret

[John Ferlan]

Add the secret object prior to the chardev tcp so the 'passwordid=' can
be added if the domain XML has a  for the chardev TLS.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_command.c|  31 ++-
 src/qemu/qemu_command.h|   1 +
 src/qemu/qemu_domain.c | 103 -
 src/qemu/qemu_domain.h |  16 +++-
 src/qemu/qemu_hotplug.c|   1 +
 src/qemu/qemu_process.c|   4 +-
 ...xml2argv-serial-tcp-tlsx509-secret-chardev.args |  38 
 ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml |  50 ++
 tests/qemuxml2argvtest.c   |  17 
 9 files changed, 254 insertions(+), 7 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f00751a..9a14b44 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -695,6 +695,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
  * @tlspath: path to the TLS credentials
  * @listen: boolen listen for client or server setting
  * @verifypeer: boolean to enable peer verification (form of authorization)
+ * @secalias: if one exists, the alias of the security object for passwordid
  * @qemuCaps: capabilities
  * @propsret: json properties to return
  *
@@ -706,6 +707,7 @@ int
 qemuBuildTLSx509BackendProps(const char *tlspath,
  bool isListen,
  bool verifypeer,
+ const char *secalias,
  virQEMUCapsPtr qemuCaps,
  virJSONValuePtr *propsret)
 {
@@ -731,6 +733,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
  NULL) < 0)
 goto cleanup;
 
+if (secalias &&
+virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL) < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
@@ -745,6 +751,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
  * @tlspath: path to the TLS credentials
  * @listen: boolen listen for client or server setting
  * @verifypeer: boolean to enable peer verification (form of authorization)
+ * @addpasswordid: boolean to handle adding passwordid to object
  * @inalias: Alias for the parent to generate object alias
  * @qemuCaps: capabilities
  *
@@ -757,6 +764,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 const char *tlspath,
 bool isListen,
 bool verifypeer,
+bool addpasswordid,
 const char *inalias,
 virQEMUCapsPtr qemuCaps)
 {
@@ -764,11 +772,16 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 char *objalias = NULL;
 virJSONValuePtr props = NULL;
 char *tmp = NULL;
+char *secalias = NULL;
 
-if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer,
- qemuCaps, ) < 0)
+if (addpasswordid &&
+!(secalias = qemuDomainGetSecretAESAlias(inalias, false)))
 return -1;
 
+if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias,
+ qemuCaps, ) < 0)
+goto cleanup;
+
 if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias)))
 goto cleanup;
 
@@ -784,6 +797,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 virJSONValueFree(props);
 VIR_FREE(objalias);
 VIR_FREE(tmp);
+VIR_FREE(secalias);
 return ret;
 }
 
@@ -4949,6 +4963,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
 dev->data.tcp.listen,
 cfg->chardevTLSx509verify,
+!!cfg->chardevTLSx509secretUUID,
 charAlias, qemuCaps) < 0)
 goto error;
 
@@ -8546,6 +8561,18 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
 
 /* Use -chardev with -device if they are available */
 if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) {
+qemuDomainChardevPrivatePtr chardevPriv =
+QEMU_DOMAIN_CHARDEV_PRIVATE(serial);
+
+/* Add the secret object first if necessary. The
+ * secinfo is added only to a TCP serial device during
+ * qemuDomainSecretChardevPrepare. Subsequently called
+ * functions can just check the config fields */
+if (serial->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
+chardevPriv && chardevPriv->secinfo &&
+  

Re: [libvirt] [RFC] Toward a better NEWS file

[Martin Kletzander]

On Wed, Oct 19, 2016 at 03:59:37PM +0200, Andrea Bolognani wrote:

On Wed, 2016-10-19 at 15:19 +0200, Martin Kletzander wrote:

> > Why don't we simply have a NEWS file in GIT, and require that
> > non-trivial commits or patch series include an update to NEWS,
> > so the NEWS file gets populated at time the feature/bug fix
> > gets merged.
> 
> I'm strongly against adding more generated files to the
> repository; if anything, we should have *less* of them[1].
> 
> But if we required the source file, docs/news.html.in, to
> be updated along with notable changes instead, I would be
> all for it! :)
 
I understood it like this:
 
  - stop generating NEWS file
  - populate NEWS file with notable features/bug-fixes along with the
changes themselves
  - use NEWS to make nice news.html


Why would we build news.html from NEWS when we already have
a perfectly fine way to build both NEWS and news.html from
news.html.in?



I meant news.html.in of course.  But we can be updating news.html.in and
keep all the files generated as they are now.


> [1] Hello, HACKING!
 
Yeah, that's a problem, we want the plain-text HACKING to be there, but
we want the stuff to be on the web page too.  This could be fixed by
making hacking.html.in generated from HACKING and changing HACKING to
use some kind of plaint-text friendly formatted text (org, rst, md, …)
in order not to lose the metadata ;)


I was discussing this offline with Ján just yesterday.
IMHO the way forward is to basically

  * stop building HACKING from hacking.html.in
  * move README-hacking to HACKING
  * (optionally) rename hacking.html.in to
contributorguidelines.html.in - that's already the title
of the document anyway
  * improve the contents of both HACKING and hacking.html.in

I think HACKING should contain just the information required
to get from a fresh git clone to a buildable source tree, eg.
the extra steps you wouldn't have to take if you were building
from a release archive. And README-hacking is basically there
already.



OK, so you had different plans while I was just thinking how to keep the
same things in place but remove redundant duplicates from git.


-- 
Andrea Bolognani / Red Hat / Virtualization


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] xl: don't output (null) target in domxml-to-native

[John Ferlan]

On 10/19/2016 06:10 AM, Cédric Bosdonnat wrote:
> When converting a domain xml containing a CDROM device without
> any attached source, don't add a target=(null) to the libxl config
> disk definition: xen doesn't like it at all and would fail to start
> the domain.
> ---
>  src/xenconfig/xen_xl.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

After your push, make check for xlconfigtest is now broken (at least on
my Fedora run)

John
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index a06983e..db8cbf1 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -1068,7 +1068,7 @@ xenFormatXLDisk(virConfValuePtr list, 
> virDomainDiskDefPtr disk)
>  
>  /* devtype */
>  if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> -virBufferAddLit(, "devtype=cdrom,");
> +virBufferAddLit(, "devtype=cdrom");
>  
>  /*
>   * target
> @@ -1081,7 +1081,9 @@ xenFormatXLDisk(virConfValuePtr list, 
> virDomainDiskDefPtr disk)
>  if (xenFormatXLDiskSrc(disk->src, ) < 0)
>  goto cleanup;
>  
> -virBufferAsprintf(, "target=%s", target);
> +if (target) {
> +virBufferAsprintf(, ",target=%s", target);
> +}
>  
>  if (virBufferCheckError() < 0)
>  goto cleanup;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] Make UEFI firmware config simpler

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 01:14:42PM -0400, Cole Robinson wrote:
> On 10/03/2016 11:49 AM, Daniel P. Berrange wrote:
> > This series lets apps enabled UEFI for a guest by
> > simply doing
> > 
> >   
> > 
> > with the other (existing) attributes being auto-filled
> > with correct QEMU specific defaults.
> > 
> 
> Wasn't clear from the commit messages, but just listing explicitly that after
> these patches the secboot firmware can be selected with:
> 
>   
> 
> correct?

Yeah, that should work.

> I think from virt-manager's perspective this is all sufficient, though we may
> continue to use the old method since it allows us to list multiple rom+nvram
> pairs if they exist (like system packages vs the upstream binaries). But
> certainly I think this interface is what tools like oz will want, to simplify
> the 'just give me uefi' case
> 
> Still though, the ./configure option isn't very flexible and is getting pretty
> unwieldy. I figure at some point we are going to need to do something akin to
> gerd's suggested /etc/libvirt/firmware.d
> 
> https://www.redhat.com/archives/virt-tools-list/2014-September/msg00145.html

This matches up with Rich Jones' request from libguestfs POV - I think
its really a QEMU level config rather than libvirt though, eg /etc/qemu
material


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] Make UEFI firmware config simpler

[Cole Robinson]

On 10/03/2016 11:49 AM, Daniel P. Berrange wrote:
> This series lets apps enabled UEFI for a guest by
> simply doing
> 
>   
> 
> with the other (existing) attributes being auto-filled
> with correct QEMU specific defaults.
> 

Wasn't clear from the commit messages, but just listing explicitly that after
these patches the secboot firmware can be selected with:

  

correct?

I think from virt-manager's perspective this is all sufficient, though we may
continue to use the old method since it allows us to list multiple rom+nvram
pairs if they exist (like system packages vs the upstream binaries). But
certainly I think this interface is what tools like oz will want, to simplify
the 'just give me uefi' case

Still though, the ./configure option isn't very flexible and is getting pretty
unwieldy. I figure at some point we are going to need to do something akin to
gerd's suggested /etc/libvirt/firmware.d

https://www.redhat.com/archives/virt-tools-list/2014-September/msg00145.html

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 01:06:05PM -0400, Cole Robinson wrote:
> On 10/03/2016 11:49 AM, Daniel P. Berrange wrote:
> > Currently qemu.conf contains a nvram parameter which
> > lists firmware code files and the corresponding nvram
> > file path. We need to know which architecture and
> > features are associated with each firmware file for
> > future enhancement. This extends the syntax in a
> > backwards compatible manner to record this info.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  src/libvirt_private.syms   |  1 +
> >  src/qemu/qemu.conf | 14 --
> >  src/qemu/test_libvirtd_qemu.aug.in |  6 +--
> >  src/util/virfirmware.c | 94 
> > ++
> >  src/util/virfirmware.h |  7 +++
> >  5 files changed, 105 insertions(+), 17 deletions(-)
> 
> I think this should also update the --nvram config in the spec file as well

Oh good catch - i didn't realize we overrode the defaults


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Cole Robinson]

On 10/03/2016 11:49 AM, Daniel P. Berrange wrote:
> Currently qemu.conf contains a nvram parameter which
> lists firmware code files and the corresponding nvram
> file path. We need to know which architecture and
> features are associated with each firmware file for
> future enhancement. This extends the syntax in a
> backwards compatible manner to record this info.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/libvirt_private.syms   |  1 +
>  src/qemu/qemu.conf | 14 --
>  src/qemu/test_libvirtd_qemu.aug.in |  6 +--
>  src/util/virfirmware.c | 94 
> ++
>  src/util/virfirmware.h |  7 +++
>  5 files changed, 105 insertions(+), 17 deletions(-)

I think this should also update the --nvram config in the spec file as well

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] spec: Drop support for Fedora < 23

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 05:11:02PM +0200, Andrea Bolognani wrote:
> We only claim support for OSs that are still supported by the
> respective vendors, which means anything older than Fedora 23
> is out. Reword the comment a bit to highlight the criteria.
> ---
> Changes from v1:
> 
>   * drop version checks that are now obsolete (thanks Dan)
> 
>  libvirt.spec.in | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] FOSDEM 2017: Call For Proposals -- Virtualization & IaaS DevRoom

[Kashyap Chamarthy]

===

The call for proposals is now open for the Virtualization & IaaS devroom
at the upcoming FOSDEM 2017, to be hosted on February 4, 2017.

This year will mark FOSDEM’s 17th anniversary as one of the
longest-running free and open source software developer events,
attracting thousands of developers and users from all over the world.
FOSDEM will be held once again in Brussels, Belgium, on February 4 & 5,
2017.

---
Important Dates
---

Submission deadline: 18 November 2016
Acceptance notifications: 04 December 2016
Final schedule announcement: 11 December 2016
Devroom: 04 February 2017 (one day)

-
About the Devroom
-

The Virtualization & IaaS devroom will feature session topics such as
open source hypervisors and virtual machine managers such as Xen
Project, KVM, QEMU, bhyve, and VirtualBox, and
Infrastructure-as-a-Service projects such as Apache CloudStack,
OpenStack, oVirt, OpenNebula, and Ganeti.

This devroom will host presentations that focus on topics of shared
interest, such as KVM; libvirt; shared storage; virtualized networking;
cloud security; clustering and high availability; interfacing with
multiple hypervisors; hyperconverged deployments; and scaling across
hundreds or thousands of servers.

Presentations in this devroom will be aimed at developers working on
these platforms who are looking to collaborate and improve shared
infrastructure or solve common problems. We seek topics that encourage
dialog between projects and continued work post-FOSDEM.


Submit Your Proposal


All submissions must be made via the Pentabarf event planning site.

https://penta.fosdem.org/submission/FOSDEM17

If you have not used Pentabarf before, you will need to create an
account.  If you submitted proposals for FOSDEM in previous years, you
can use your existing account.

After creating the account, select Create Event to start the submission
process. Make sure to select Virtualisation and IaaS devroom from the
Track list. Please fill out all the required fields, and provide a
meaningful abstract and description of your proposed session.

-
Submission Guidelines
-

- We expect more proposals than we can possibly accept, so it is vitally
  important that you submit your proposal on or before the deadline.
  Late submissions are unlikely to be considered.

- All presentation slots are 45 minutes, with 35 minutes planned for
  presentations, and 10 minutes for Q

- All presentations will be recorded and made available under Creative
  Commons licenses. In the Submission notes field, please indicate that
  you agree that your presentation will be licensed under the
  CC-By-SA-4.0 or CC-By-4.0 license and that you agree to have your
  presentation recorded. For example:

"If my presentation is accepted for FOSDEM, I hereby agree to
license all recordings, slides, and other associated materials under
the Creative Commons Attribution Share-Alike 4.0 International
License.  Sincerely, ."

- In the Submission notes field, please also confirm that if your talk
  is accepted, you will be able to attend FOSDEM and deliver your
  presentation. We will not consider proposals from prospective speakers
  who are unsure whether they will be able to secure funds for travel
  and lodging to attend FOSDEM. (Sadly, we are not able to offer travel
  funding for prospective speakers.)

---
Call for Volunteers
---

We are also looking for volunteers to help run the devroom. We need
assistance watching time for the speakers, and helping with video for
the devroom. Please contact Brian Proffitt (bkp at redhat.com), for more
information.

--
Questions?
--

If you have any questions about this devroom, please send your questions
to our devroom mailing list. 

iaas-virt-devr...@lists.fosdem.org

You can also subscribe to the list to receive updates about important
dates, session announcements, and to connect with other attendees.

===

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] spec: Drop support for Fedora < 23

[Andrea Bolognani]

We only claim support for OSs that are still supported by the
respective vendors, which means anything older than Fedora 23
is out. Reword the comment a bit to highlight the criteria.
---
Changes from v1:

  * drop version checks that are now obsolete (thanks Dan)

 libvirt.spec.in | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 00b95b8..545990c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1,9 +1,10 @@
 # -*- rpm-spec -*-
 
-# This spec file assumes you are building for Fedora 20 or newer,
-# or for RHEL 6 or newer. It may need some tweaks for other distros.
+# This spec file assumes you are building on a Fedora or RHEL version
+# that's still supported by the vendor: that means Fedora 23 or newer,
+# or RHEL 6 or newer. It may need some tweaks for other distros.
 # If neither fedora nor rhel was defined, try to guess them from dist
-%if (0%{?fedora} && 0%{?fedora} >= 20) || (0%{?rhel} && 0%{?rhel} >= 6)
+%if (0%{?fedora} && 0%{?fedora} >= 23) || (0%{?rhel} && 0%{?rhel} >= 6)
 %define supported_platform 1
 %else
 %define supported_platform 0
@@ -167,7 +168,7 @@
 %endif
 
 # Enable wireshark plugins for all distros shipping libvirt 1.2.2 or newer
-%if 0%{?fedora} >= 21
+%if 0%{?fedora}
 %define with_wireshark 0%{!?_without_wireshark:1}
 %endif
 
@@ -209,7 +210,7 @@
 %if 0%{?fedora} >= 25
 %define tls_priority "@LIBVIRT,SYSTEM"
 %else
-%if 0%{?fedora} >= 21
+%if 0%{?fedora}
 %define tls_priority "@SYSTEM"
 %else
 %define tls_priority "NORMAL"
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] spec: Drop support for Fedora < 23

[Andrea Bolognani]

On Wed, 2016-10-19 at 15:58 +0100, Daniel P. Berrange wrote:
> On Wed, Oct 19, 2016 at 04:51:27PM +0200, Andrea Bolognani wrote:
> > 
> > We only claim support for OSs that are still supported by the
> > respective vendors, which means anything older than Fedora 23
> > is out. Reword the comment a bit to highlight the criteria.
> > ---
> >  libvirt.spec.in | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index 00b95b8..25dc31d 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -1,9 +1,10 @@
> >  # -*- rpm-spec -*-
> >  
> > -# This spec file assumes you are building for Fedora 20 or newer,
> > -# or for RHEL 6 or newer. It may need some tweaks for other distros.
> > +# This spec file assumes you are building on a Fedora or RHEL version
> > +# that's still supported by the vendor: that means Fedora 23 or newer,
> > +# or RHEL 6 or newer. It may need some tweaks for other distros.
> >  # If neither fedora nor rhel was defined, try to guess them from dist
> > -%if (0%{?fedora} && 0%{?fedora} >= 20) || (0%{?rhel} && 0%{?rhel} >= 6)
> > +%if (0%{?fedora} && 0%{?fedora} >= 23) || (0%{?rhel} && 0%{?rhel} >= 6)
> 
> If you change this version, you should be updating/removing conditional
> checks in the file which use versions < 23

Right you are. v2 coming in a minute :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] spec: Drop support for Fedora < 23

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 04:51:27PM +0200, Andrea Bolognani wrote:
> We only claim support for OSs that are still supported by the
> respective vendors, which means anything older than Fedora 23
> is out. Reword the comment a bit to highlight the criteria.
> ---
>  libvirt.spec.in | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 00b95b8..25dc31d 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1,9 +1,10 @@
>  # -*- rpm-spec -*-
>  
> -# This spec file assumes you are building for Fedora 20 or newer,
> -# or for RHEL 6 or newer. It may need some tweaks for other distros.
> +# This spec file assumes you are building on a Fedora or RHEL version
> +# that's still supported by the vendor: that means Fedora 23 or newer,
> +# or RHEL 6 or newer. It may need some tweaks for other distros.
>  # If neither fedora nor rhel was defined, try to guess them from dist
> -%if (0%{?fedora} && 0%{?fedora} >= 20) || (0%{?rhel} && 0%{?rhel} >= 6)
> +%if (0%{?fedora} && 0%{?fedora} >= 23) || (0%{?rhel} && 0%{?rhel} >= 6)

If you change this version, you should be updating/removing conditional
checks in the file which use versions < 23

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Added the description for Qt Virtual machine manager and Qt Remote Viewer as libvirt related applications.

[F1ash]

---
 docs/apps.html.in | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 1a138b3..3098b73 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -208,6 +208,17 @@
 to remote consoles supporting the VNC protocol. Also provides
 an optional mozilla browser plugin.
   
+  http://f1ash.github.io/qt-virt-manager;>qt-virt-manager
+  
+The Qt GUI for create and control VMs and another virtual entities
+(aka networks, storages, interfaces, secrets, network filters).
+Contains integrated LXC/SPICE/VNC viewer for accessing the graphical or
+text console associated with a virtual machine or container.
+  
+  http://f1ash.github.io/qt-virt-manager;>qt-remote-viewer
+  
+The Qt VNC/SPICE viewer for access to remote desktops or VMs.
+  
 
 
 Infrastructure as a Service (IaaS)
-- 
2.10.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] spec: Drop support for Fedora < 23

[Andrea Bolognani]

We only claim support for OSs that are still supported by the
respective vendors, which means anything older than Fedora 23
is out. Reword the comment a bit to highlight the criteria.
---
 libvirt.spec.in | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 00b95b8..25dc31d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1,9 +1,10 @@
 # -*- rpm-spec -*-
 
-# This spec file assumes you are building for Fedora 20 or newer,
-# or for RHEL 6 or newer. It may need some tweaks for other distros.
+# This spec file assumes you are building on a Fedora or RHEL version
+# that's still supported by the vendor: that means Fedora 23 or newer,
+# or RHEL 6 or newer. It may need some tweaks for other distros.
 # If neither fedora nor rhel was defined, try to guess them from dist
-%if (0%{?fedora} && 0%{?fedora} >= 20) || (0%{?rhel} && 0%{?rhel} >= 6)
+%if (0%{?fedora} && 0%{?fedora} >= 23) || (0%{?rhel} && 0%{?rhel} >= 6)
 %define supported_platform 1
 %else
 %define supported_platform 0
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 02:52:20PM +0100, Richard W.M. Jones wrote:
> On Wed, Oct 19, 2016 at 02:22:19PM +0100, Daniel P. Berrange wrote:
> > Oh, but that's outside scope of libvirt then - we're not looking to
> > expose APIs to help people run QEMU directly.
> 
> Well, yes, but that doesn't mean we cannot cooperate.  These wouldn't
> be libvirt APIs, just a standard configuration file somewhere listing
> the UEFI binaries.  Or perhaps more simply, a more sensible naming of
> UEFI binaries so they're always in a standard location with standard
> names.

I think this would be something QEMU has to define. Unfortunately I
don't think simple filenaming conventions will work for UEFI, since
depending on which QEMU features you turn on you need to pick
different images - indeed we already have every distro using different
path names for UEFI, which is why libvirt already has this config
parameter :-( QEMU could define some file format with mapping info,
that listed which UEFI binaries are valid for what QEMU features.
QEMU could ship a default file, and OS distro vendors could as/if
needed

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] conf: add support for choosing firmware type

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 01:09:56PM +0100, Richard W.M. Jones wrote:
> On Mon, Oct 03, 2016 at 04:49:48PM +0100, Daniel P. Berrange wrote:
> > Currently if you want to enable UEFI firmware for a guest
> > you need to know about the hypervisor platform specific
> > firmware path. This does not even work for all platforms,
> > as hypervisors like VMWare don't expose UEFI as a path,
> > they just have a direct config option for turning it on
> > or off.
> > 
> > This adds ability to use the much simpler:
> > 
> >   
> > 
> > to choose the different firmware types.
> 
> Although not in the scope of this patch, virt-v2v would really like
> the ESX driver in libvirt to return this data for existing guests.  I
> believe there is an open RFE about that somewhere.

Yep, it should be possile to wire this up to ESX driver based on
the existing data in VMX format.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] Toward a better NEWS file

[Andrea Bolognani]

On Wed, 2016-10-19 at 15:19 +0200, Martin Kletzander wrote:
> > > Why don't we simply have a NEWS file in GIT, and require that
> > > non-trivial commits or patch series include an update to NEWS,
> > > so the NEWS file gets populated at time the feature/bug fix
> > > gets merged.
> > 
> > I'm strongly against adding more generated files to the
> > repository; if anything, we should have *less* of them[1].
> > 
> > But if we required the source file, docs/news.html.in, to
> > be updated along with notable changes instead, I would be
> > all for it! :)
> 
> I understood it like this:
> 
>  - stop generating NEWS file
>  - populate NEWS file with notable features/bug-fixes along with the
>changes themselves
>  - use NEWS to make nice news.html

Why would we build news.html from NEWS when we already have
a perfectly fine way to build both NEWS and news.html from
news.html.in?

> > [1] Hello, HACKING!
> 
> Yeah, that's a problem, we want the plain-text HACKING to be there, but
> we want the stuff to be on the web page too.  This could be fixed by
> making hacking.html.in generated from HACKING and changing HACKING to
> use some kind of plaint-text friendly formatted text (org, rst, md, …)
> in order not to lose the metadata ;)

I was discussing this offline with Ján just yesterday.
IMHO the way forward is to basically

  * stop building HACKING from hacking.html.in
  * move README-hacking to HACKING
  * (optionally) rename hacking.html.in to
contributorguidelines.html.in - that's already the title
of the document anyway
  * improve the contents of both HACKING and hacking.html.in

I think HACKING should contain just the information required
to get from a fresh git clone to a buildable source tree, eg.
the extra steps you wouldn't have to take if you were building
from a release archive. And README-hacking is basically there
already.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Richard W.M. Jones]

On Wed, Oct 19, 2016 at 02:22:19PM +0100, Daniel P. Berrange wrote:
> Oh, but that's outside scope of libvirt then - we're not looking to
> expose APIs to help people run QEMU directly.

Well, yes, but that doesn't mean we cannot cooperate.  These wouldn't
be libvirt APIs, just a standard configuration file somewhere listing
the UEFI binaries.  Or perhaps more simply, a more sensible naming of
UEFI binaries so they're always in a standard location with standard
names.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/1] reset vcpu pin info from config for zero mask

[Konstantin Neumoin]

The option for removing vcpu pinning information from config was added
in:
'7ea9778 vcpupin: add vcpupin resetting feature to qemu driver'
and removed in:
'a02a161 qemu: libxl: vcpupin: Don't reset pinning when pinning to all pcpus'
by some reasons.

So, for now there is no way to remove vcpu pinning from config.
This patch returns options for remove vcpu/emulator pinning settings
from both configs if zero mask(mask filled by zeros) was specified.

Signed-off-by: Konstantin Neumoin 
---
 src/qemu/qemu_driver.c | 74 +++---
 1 file changed, 52 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bec7a38..7aa64a4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4969,7 +4969,8 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
   virQEMUDriverConfigPtr cfg,
   virBitmapPtr cpumap)
 {
-virBitmapPtr tmpmap = NULL;
+virBitmapPtr effective_cpumap = NULL;
+virBitmapPtr allcpu_map = NULL;
 virDomainVcpuInfoPtr vcpuinfo;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virCgroupPtr cgroup_vcpu = NULL;
@@ -4980,6 +4981,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
 int eventNparams = 0;
 int eventMaxparams = 0;
 int ret = -1;
+int hostcpus = 0;
 
 if (!qemuDomainHasVcpuPids(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
@@ -4994,29 +4996,38 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
 goto cleanup;
 }
 
-if (!(tmpmap = virBitmapNewCopy(cpumap)))
-goto cleanup;
+if (vcpuinfo->online) {
+if (cpumap) {
+effective_cpumap = cpumap;
+} else if (def->cpumask) {
+effective_cpumap = def->cpumask;
+} else {
+if ((hostcpus = nodeGetCPUCount(NULL)) < 0)
+goto cleanup;
 
-if (!(str = virBitmapFormat(cpumap)))
-goto cleanup;
+if (!(allcpu_map = virBitmapNew(hostcpus)))
+goto cleanup;
+virBitmapSetAll(allcpu_map);
+effective_cpumap = allcpu_map;
+}
 
-if (vcpuinfo->online) {
 /* Configure the corresponding cpuset cgroup before set affinity. */
 if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
 if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu,
false, _vcpu) < 0)
 goto cleanup;
-if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
+if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, effective_cpumap) < 0)
 goto cleanup;
 }
 
-if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), cpumap) < 0)
+if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), 
effective_cpumap) < 0)
 goto cleanup;
 }
 
 virBitmapFree(vcpuinfo->cpumask);
-vcpuinfo->cpumask = tmpmap;
-tmpmap = NULL;
+vcpuinfo->cpumask = NULL;
+if (cpumap && !(vcpuinfo->cpumask = virBitmapNewCopy(cpumap)))
+goto cleanup;
 
 if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 
0)
 goto cleanup;
@@ -5026,6 +5037,9 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
 goto cleanup;
 }
 
+if (!(str = virBitmapFormat(effective_cpumap)))
+goto cleanup;
+
 if (virTypedParamsAddString(, ,
 , paramField, str) < 0)
 goto cleanup;
@@ -5035,7 +5049,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
 ret = 0;
 
  cleanup:
-virBitmapFree(tmpmap);
+virBitmapFree(allcpu_map);
 virCgroupFree(_vcpu);
 VIR_FREE(str);
 qemuDomainEventQueue(driver, event);
@@ -5089,9 +5103,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
 goto endjob;
 
 if (virBitmapIsAllClear(pcpumap)) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
-   _("Empty cpu list for pinning"));
-goto endjob;
+virBitmapFree(pcpumap);
+pcpumap = NULL;
 }
 
 if (def &&
@@ -5177,12 +5190,15 @@ qemuDomainPinEmulator(virDomainPtr dom,
 int ret = -1;
 qemuDomainObjPrivatePtr priv;
 virBitmapPtr pcpumap = NULL;
+virBitmapPtr allcpu_map = NULL;
+virBitmapPtr effective_pcpumap = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
 virObjectEventPtr event = NULL;
 char *str = NULL;
 virTypedParameterPtr eventParams = NULL;
 int eventNparams = 0;
 int eventMaxparams = 0;
+int hostcpus = 0;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -5207,18 +5223,31 @@ qemuDomainPinEmulator(virDomainPtr dom,
 goto endjob;
 
 if (virBitmapIsAllClear(pcpumap)) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
-   _("Empty cpu list for pinning"));
-goto endjob;
+virBitmapFree(pcpumap);
+pcpumap = NULL;
 }
 
 if 

[libvirt] [PATCH] Allow virtio-console on PPC64

[Shivaprasad G Bhat]

virQEMUCapsSupportsChardev existing checks returns true
for spapr-vty alone. Instead verify spapr-vty validity
and let the logic to return true for other device types
so that virtio-console passes.

The non-pseries machines dont have spapr-vio-bus. So, the
function always returned false for them before.

Fixes - https://bugzilla.redhat.com/show_bug.cgi?id=1257813

Signed-off-by: Shivaprasad G Bhat 
---
 src/qemu/qemu_capabilities.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6eee85d..784496b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4286,9 +4286,12 @@ virQEMUCapsSupportsChardev(const virDomainDef *def,
 return false;
 
 if ((def->os.arch == VIR_ARCH_PPC) || ARCH_IS_PPC64(def->os.arch)) {
+if (!qemuDomainMachineIsPSeries(def))
+return false;
 /* only pseries need -device spapr-vty with -chardev */
-return (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
-chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO);
+if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+chr->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
+return false;
 }
 
 if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != 
VIR_ARCH_AARCH64))

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 02:13:52PM +0100, Richard W.M. Jones wrote:
> On Wed, Oct 19, 2016 at 01:18:07PM +0100, Daniel P. Berrange wrote:
> > On Wed, Oct 19, 2016 at 01:07:25PM +0100, Richard W.M. Jones wrote:
> > > Unfortunately it's a case of so near and yet so far.  You're proposing
> > > this essentially static and non-secret data be stored in
> > > /etc/libvirt/qemu.conf, which is not readable as non-root.  virt-v2v
> > > (which can run as non-root) would still need to store a duplicate copy
> > > of the data.
> > 
> > What does v2v need the mapping data for ?  Any use case needs to be
> > addressed via the APIs, not having apps poke at libvirt private
> > config files.
> 
> Well the use is fairly narrow (and not present in RHEL).  If you use
> `-o qemu' mode, then virt-v2v will write a shell script that invokes
> qemu directly.  To do this for UEFI guests it needs to know the right
> firmware paths to use.

Oh, but that's outside scope of libvirt then - we're not looking to
expose APIs to help people run QEMU directly.

> There's also the issue of writing guests out to old versions of
> libvirt, but I'm not too worried about that since we could make the
> switch after we are sure the minimum version of libvirt supports
>  everywhere.
> 
> Libguestfs itself also uses UEFI paths on aarch64 when backend = direct
> to run the appliance.

Ok, but that's outside scope of libvirt again.

> We could query the data through an API, I suppose, although that
> assumes libvirt is present.  Could this information be stored
> somewhere outside libvirt?  It's useful for people running qemu
> directly.

With other BIOS files, QEMU has a location it expects them to be at,
but this isn't applicable to UEFI since none of the QEMU machine types
default to using UEFI. If some machine types did start defaulting to
UEFI, then QEMU would have to define this in some manner, as it does
for other BIOS. There's the slight extra complication in that there
are multiple possible BIOS files for UEFI depending on featureset
you want to use :-(

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] Toward a better NEWS file

[Martin Kletzander]

On Wed, Oct 19, 2016 at 02:54:06PM +0200, Andrea Bolognani wrote:

On Wed, 2016-10-19 at 12:58 +0100, Daniel P. Berrange wrote:

Why don't we simply have a NEWS file in GIT, and require that
non-trivial commits or patch series include an update to NEWS,
so the NEWS file gets populated at time the feature/bug fix
gets merged.


I'm strongly against adding more generated files to the
repository; if anything, we should have *less* of them[1].

But if we required the source file, docs/news.html.in, to
be updated along with notable changes instead, I would be
all for it! :)



I understood it like this:

- stop generating NEWS file
- populate NEWS file with notable features/bug-fixes along with the
  changes themselves
- use NEWS to make nice news.html



[1] Hello, HACKING!


Yeah, that's a problem, we want the plain-text HACKING to be there, but
we want the stuff to be on the web page too.  This could be fixed by
making hacking.html.in generated from HACKING and changing HACKING to
use some kind of plaint-text friendly formatted text (org, rst, md, …)
in order not to lose the metadata ;)


-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Richard W.M. Jones]

On Wed, Oct 19, 2016 at 01:18:07PM +0100, Daniel P. Berrange wrote:
> On Wed, Oct 19, 2016 at 01:07:25PM +0100, Richard W.M. Jones wrote:
> > Unfortunately it's a case of so near and yet so far.  You're proposing
> > this essentially static and non-secret data be stored in
> > /etc/libvirt/qemu.conf, which is not readable as non-root.  virt-v2v
> > (which can run as non-root) would still need to store a duplicate copy
> > of the data.
> 
> What does v2v need the mapping data for ?  Any use case needs to be
> addressed via the APIs, not having apps poke at libvirt private
> config files.

Well the use is fairly narrow (and not present in RHEL).  If you use
`-o qemu' mode, then virt-v2v will write a shell script that invokes
qemu directly.  To do this for UEFI guests it needs to know the right
firmware paths to use.

There's also the issue of writing guests out to old versions of
libvirt, but I'm not too worried about that since we could make the
switch after we are sure the minimum version of libvirt supports
 everywhere.

Libguestfs itself also uses UEFI paths on aarch64 when backend = direct
to run the appliance.

We could query the data through an API, I suppose, although that
assumes libvirt is present.  Could this information be stored
somewhere outside libvirt?  It's useful for people running qemu
directly.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] Toward a better NEWS file

[Martin Kletzander]

On Wed, Oct 19, 2016 at 12:58:55PM +0100, Daniel P. Berrange wrote:

On Wed, Oct 19, 2016 at 01:53:41PM +0200, Andrea Bolognani wrote:

Hi,

there's an idea that has been kicking around in my head for
a while, and I'd like to share it with the list to gather
some feedback before I forget about it :)

Right now, each entry in our NEWS file contains what is
basically the output of

  git log \
  --pretty=oneline \
  vX.Y-1.0..vX.Y.0

with the commits organized somewhat arbitrarily into a bunch
of sections with partially overlapping scopes.

I believe the current form is less than useful, as it is too
detailed for end users and distro packagers, who only care
about the high-level user visible changes, and not detailed
enough for developers, who are always going to refer to the
proper git log anyway. Moreover, we ship a ChangeLog file
that contains very detailed information already.

Ideally, the NEWS file would contain a summary of notable
changes (new APIs, significantly improved features, etc.)
laid out in an easy to digest fashion, such that people who
are not knee-deep into libvirt development can grasp them
and hopefully get excited about upgrading to our latest and
greatest release :)

Of course, it would take an insane amount of time for any
single one of us to turn the git log into such a document,
and the result would still be sub-par because we simply
can't expect anyone to have full insight in every single
change to the project.

My solution for this is to ask the people with the most
knowledge of the changes - the authors themselves!

The workflow I'm envisioning would look like this:

  * DV, at the same time as he announces that libvirt has
entered freeze, will put out a Call for NEWS and ask
people who have contributed code to the upcoming release
to post a summary of their changes

  * the authors will go over

  git log \
  --author=$(git config user.email) \
  vX.Y-1.0..master

and come up with a short (one-three sentences) summary
for each of the changes, if they are notable. Commits
that are part of a larger series would not be described
on their own: a short summary of the series would be
used instead, akin to the one you would put in your
cover letter.

To give a practical example: I've mostly been busy with
reviews this cycle, but if I were to go over my commits
since v2.3.0 right now I would write something like

  * Bug fix: don't restart libvirt-guests.service when
libvirtd.service is restarted

for commit 2273bbd, and omit both 61e1014 and a0da413 as
they're neither notable enough on their own, nor part of
a larger series: we'll always have a "various bug fixes
and improvements" bullet point in a NEWS file entry to
take care of that kind of small cleanups and improvements.

  * the authors would post the resulting summaries to the
list. We could simply post them as regular patches to
docs/news.html.in (potentially without requiring review
before pushing them), or post them as plain text and have
someone collect them and prepare a single commit

  * DV will tag the release and push the tarballs, and
everyone will be able to enjoy the NEWS file :)

Some light editoral work might be needed throughout the
process, eg. fix typos or post one or two reminders during
the freeze: I volunteer to take such tasks upon myself.



I like the idea, but going back to my patches with sometimes as less as
2-day notice, and also requiring bunch of people to do it at the same
time feels weird.


I'm looking forward to feedback about this idea, especially
from people who might be part of any community where anything
like this is already happening.


Why don't we simply have a NEWS file in GIT, and require that
non-trivial commits or patch series include an update to NEWS,
so the NEWS file gets populated at time the feature/bug fix
gets merged.



I was thinking something along the lines of this ^^.  With the exception
that it would be a file that gets used by DV at the time of release and
then cleared for the next one.  Traditional NEWS file works for me too,
though.


Regards,
Daniel
--
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] Toward a better NEWS file

[Andrea Bolognani]

On Wed, 2016-10-19 at 12:58 +0100, Daniel P. Berrange wrote:
> Why don't we simply have a NEWS file in GIT, and require that
> non-trivial commits or patch series include an update to NEWS,
> so the NEWS file gets populated at time the feature/bug fix
> gets merged.

I'm strongly against adding more generated files to the
repository; if anything, we should have *less* of them[1].

But if we required the source file, docs/news.html.in, to
be updated along with notable changes instead, I would be
all for it! :)


[1] Hello, HACKING!
-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 4/6] remote: expose a new libssh transport

[Pino Toscano]

Implement in virtNetClient and VirNetSocket the needed functions to
expose a new libssh transport, providing all the options that the
libssh2 transport supports.
---
 docs/remote.html.in|  35 ++---
 src/remote/remote_driver.c |  41 +++
 src/rpc/virnetclient.c | 118 ++
 src/rpc/virnetclient.h |  13 
 src/rpc/virnetsocket.c | 179 +
 src/rpc/virnetsocket.h |  13 
 6 files changed, 387 insertions(+), 12 deletions(-)

diff --git a/docs/remote.html.in b/docs/remote.html.in
index 4c3012f..c28a505 100644
--- a/docs/remote.html.in
+++ b/docs/remote.html.in
@@ -145,6 +145,13 @@ of the OpenSSH binary. This transport uses the libvirt 
authentication callback f
 all ssh authentication calls and therefore supports keyboard-interactive 
authentication
 even with graphical management applications. As with the classic ssh transport
 netcat is required on the remote side.
+  libssh
+   Transport over the SSH protocol using
+  http://libssh.org/; title="libssh homepage">libssh instead
+of the OpenSSH binary. This transport uses the libvirt authentication callback 
for
+all ssh authentication calls and therefore supports keyboard-interactive 
authentication
+even with graphical management applications. As with the classic ssh transport
+netcat is required on the remote side.
 
 
 The default transport, if no other is specified, is tls.
@@ -192,6 +199,9 @@ settings.
 
qemu+libssh2://user@host/system?known_hosts=/home/user/.ssh/known_hosts
 
 Connect to a remote host using a ssh connection with the libssh2 driver
 and use a different known_hosts file.
+qemu+libssh://user@host/system?known_hosts=/home/user/.ssh/known_hosts
 
+Connect to a remote host using a ssh connection with the libssh driver
+and use a different known_hosts file.
 
 
   Extra parameters
@@ -260,7 +270,7 @@ Note that parameter values must be
 
   socket
 
- unix, ssh, libssh2 
+ unix, ssh, libssh2, libssh 
 
   The path to the Unix domain socket, which overrides the
   compiled-in default.  For ssh transport, this is passed to
@@ -275,7 +285,7 @@ Note that parameter values must be
 
   netcat
 
- ssh, libssh2 
+ ssh, libssh2, libssh 
 
   The name of the netcat command on the remote machine.
   The default is nc.  For ssh transport, libvirt
@@ -300,7 +310,7 @@ Note that parameter values must be
 
   keyfile
 
- ssh, libssh2 
+ ssh, libssh2, libssh 
 
   The name of the private key file to use to authentication to the remote
   machine.  If this option is not used the default keys are used.
@@ -368,14 +378,15 @@ Note that parameter values must be
 
   known_hosts
 
- libssh2 
-
-  Path to the known_hosts file to verify the host key against. LibSSH2
-  supports OpenSSH-style known_hosts files, although it does not support
-  all key types, so using files created by the OpenSSH binary may result
-  into truncating the known_hosts file. It's recommended to use the default
-  known_hosts file is located in libvirt's client local configuration
-  directory e.g.: ~/.config/libvirt/known_hosts. Note: Use absolute paths.
+ libssh2, libssh 
+
+  Path to the known_hosts file to verify the host key against. LibSSH2 and
+  libssh support OpenSSH-style known_hosts files, although LibSSH2 does not
+  support all key types, so using files created by the OpenSSH binary may
+  result into truncating the known_hosts file. Thus, with LibSSH2 it's
+  recommended to use the default known_hosts file is located in libvirt's
+  client local configuration directory e.g.: ~/.config/libvirt/known_hosts.
+  Note: Use absolute paths.
 
   
   
@@ -386,7 +397,7 @@ Note that parameter values must be
 
   sshauth
 
- libssh2 
+ libssh2, libssh 
 
   A comma separated list of authentication methods to use. Default (is
   "agent,privkey,keyboard-interactive". The order of the methods is preserved.
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a3cd7cd..db2bdd4 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -673,6 +673,7 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
  *   - xxx:///-> UNIX domain socket
  *   - xxx+ssh:///-> SSH connection (legacy)
  *   - xxx+libssh2:///-> SSH connection (using libssh2)
+ *   - xxx+libssh:/// -> SSH connection (using libssh)
  */
 static int
 doRemoteOpen(virConnectPtr conn,
@@ -689,6 +690,7 @@ doRemoteOpen(virConnectPtr conn,
 trans_libssh2,
 trans_ext,
 trans_tcp,
+trans_libssh,
 } transport;
 #ifndef WIN32
 char *daemonPath = NULL;
@@ -736,6 +738,8 @@ doRemoteOpen(virConnectPtr conn,
 

[libvirt] [PATCH v3 3/6] libssh_transport: add new libssh-based transport

[Pino Toscano]

Implement a new libssh transport, which uses libssh to communicate with
remote hosts, and add all the build system stuff (search of libssh,
private symbols, etc) to built it.

This new transport supports all the common ssh authentication methods,
making use of libvirt's auth callbacks for interaction with the user.
---
 config-post.h |2 +
 configure.ac  |3 +
 m4/virt-libssh.m4 |   26 +
 po/POTFILES.in|1 +
 src/Makefile.am   |   21 +-
 src/libvirt_libssh.syms   |   22 +
 src/rpc/virnetlibsshsession.c | 1458 +
 src/rpc/virnetlibsshsession.h |   80 +++
 8 files changed, 1611 insertions(+), 2 deletions(-)
 create mode 100644 m4/virt-libssh.m4
 create mode 100644 src/libvirt_libssh.syms
 create mode 100644 src/rpc/virnetlibsshsession.c
 create mode 100644 src/rpc/virnetlibsshsession.h

diff --git a/config-post.h b/config-post.h
index 6eb63db..090cc28 100644
--- a/config-post.h
+++ b/config-post.h
@@ -36,6 +36,7 @@
 # undef WITH_DTRACE_PROBES
 # undef WITH_GNUTLS
 # undef WITH_GNUTLS_GCRYPT
+# undef WITH_LIBSSH
 # undef WITH_MACVTAP
 # undef WITH_NUMACTL
 # undef WITH_SASL
@@ -60,6 +61,7 @@
 # undef WITH_DTRACE_PROBES
 # undef WITH_GNUTLS
 # undef WITH_GNUTLS_GCRYPT
+# undef WITH_LIBSSH
 # undef WITH_MACVTAP
 # undef WITH_NUMACTL
 # undef WITH_SASL
diff --git a/configure.ac b/configure.ac
index dfc536f..f526f41 100644
--- a/configure.ac
+++ b/configure.ac
@@ -218,6 +218,7 @@ if test "$with_remote" = "no" ; then
   with_gnutls=no
   with_ssh2=no
   with_sasl=no
+  with_libssh=no
 fi
 # Stateful drivers are useful only when building the daemon.
 if test "$with_libvirtd" = "no" ; then
@@ -260,6 +261,7 @@ LIBVIRT_CHECK_UDEV
 LIBVIRT_CHECK_WIRESHARK
 LIBVIRT_CHECK_NSS
 LIBVIRT_CHECK_YAJL
+LIBVIRT_CHECK_LIBSSH
 
 AC_MSG_CHECKING([for CPUID instruction])
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
@@ -2786,6 +2788,7 @@ LIBVIRT_RESULT_DBUS
 LIBVIRT_RESULT_FUSE
 LIBVIRT_RESULT_GLUSTER
 LIBVIRT_RESULT_HAL
+LIBVIRT_RESULT_LIBSSH
 LIBVIRT_RESULT_NETCF
 LIBVIRT_RESULT_NUMACTL
 LIBVIRT_RESULT_OPENWSMAN
diff --git a/m4/virt-libssh.m4 b/m4/virt-libssh.m4
new file mode 100644
index 000..88ece21
--- /dev/null
+++ b/m4/virt-libssh.m4
@@ -0,0 +1,26 @@
+dnl The libssh.so library
+dnl
+dnl Copyright (C) 2016 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl .
+dnl
+
+AC_DEFUN([LIBVIRT_CHECK_LIBSSH],[
+  LIBVIRT_CHECK_PKG([LIBSSH], [libssh], [0.7])
+])
+
+AC_DEFUN([LIBVIRT_RESULT_LIBSSH],[
+  LIBVIRT_RESULT_LIB([LIBSSH])
+])
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 1469240..c8905f7 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -144,6 +144,7 @@ src/rpc/virnetclient.c
 src/rpc/virnetclientprogram.c
 src/rpc/virnetclientstream.c
 src/rpc/virnetdaemon.c
+src/rpc/virnetlibsshsession.c
 src/rpc/virnetmessage.c
 src/rpc/virnetsaslcontext.c
 src/rpc/virnetserver.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 8ee5567..4a6ccf3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2126,6 +2126,12 @@ else ! WITH_ATOMIC_OPS_PTHREAD
 SYM_FILES += $(srcdir)/libvirt_atomic.syms
 endif ! WITH_ATOMIC_OPS_PTHREAD
 
+if WITH_LIBSSH
+USED_SYM_FILES += $(srcdir)/libvirt_libssh.syms
+else ! WITH_LIBSSH
+SYM_FILES += $(srcdir)/libvirt_libssh.syms
+endif ! WITH_LIBSSH
+
 EXTRA_DIST += \
libvirt_public.syms \
libvirt_lxc.syms\
@@ -2203,7 +2209,8 @@ libvirt_admin_la_CFLAGS += \
$(YAJL_CFLAGS)  \
$(SSH2_CFLAGS)  \
$(SASL_CFLAGS)  \
-   $(GNUTLS_CFLAGS)
+   $(GNUTLS_CFLAGS)\
+   $(LIBSSH_CFLAGS)
 
 libvirt_admin_la_LIBADD += \
$(CAPNG_LIBS)   \
@@ -2212,7 +2219,8 @@ libvirt_admin_la_LIBADD += \
$(LIBXML_LIBS)  \
$(SSH2_LIBS)\
$(SASL_LIBS)\
-   $(GNUTLS_LIBS)
+   $(GNUTLS_LIBS)  \
+   $(LIBSSH_LIBS)
 
 ADMIN_SYM_FILES = $(srcdir)/libvirt_admin_private.syms
 
@@ -2789,16 +2797,25 @@ else ! WITH_SASL
 EXTRA_DIST += \
rpc/virnetsaslcontext.h rpc/virnetsaslcontext.c
 endif ! WITH_SASL

[libvirt] [PATCH v3 6/6] docs: fix default value for sshauth option of libssh2/libssh

[Pino Toscano]

Both transports include "password" in their default authentication
methods.
---
 docs/remote.html.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/remote.html.in b/docs/remote.html.in
index c28a505..9d3c2ec 100644
--- a/docs/remote.html.in
+++ b/docs/remote.html.in
@@ -400,8 +400,8 @@ Note that parameter values must be
  libssh2, libssh 
 
   A comma separated list of authentication methods to use. Default (is
-  "agent,privkey,keyboard-interactive". The order of the methods is preserved.
-  Some methods may require additional parameters.
+  "agent,privkey,password,keyboard-interactive". The order of the methods
+  is preserved. Some methods may require additional parameters.
 
   
   
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 5/6] spec: enable libssh transport on Fedora

[Pino Toscano]

---
 libvirt.spec.in | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 00b95b8..99e23a4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -79,6 +79,7 @@
 %define with_firewalld 0%{!?_without_firewalld:0}
 %define with_libssh2   0%{!?_without_libssh2:0}
 %define with_wireshark 0%{!?_without_wireshark:0}
+%define with_libssh0%{!?_without_libssh:0}
 %define with_pm_utils  1
 
 # Finally set the OS / architecture specific special cases
@@ -171,6 +172,11 @@
 %define with_wireshark 0%{!?_without_wireshark:1}
 %endif
 
+# Enable libssh transport for new enough distros
+%if 0%{?fedora}
+%define with_libssh 0%{!?_without_libssh:1}
+%endif
+
 
 %if %{with_qemu} || %{with_lxc} || %{with_uml}
 # numad is used to manage the CPU and memory placement dynamically,
@@ -417,6 +423,10 @@ BuildRequires: wireshark-devel >= 1.12.1
 %endif
 %endif
 
+%if %{with_libssh}
+BuildRequires: libssh-devel >= 0.7.0
+%endif
+
 Provides: bundled(gnulib)
 
 %description
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 2/6] virerror: add error for libssh transport

[Pino Toscano]

Add a new error domain and number for a new libssh-based transport.
---
 include/libvirt/virterror.h | 2 ++
 src/util/virerror.c | 9 -
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index efe83aa..2efee8f 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -131,6 +131,7 @@ typedef enum {
 VIR_FROM_XENXL = 64,/* Error from Xen xl config code */
 
 VIR_FROM_PERF = 65, /* Error from perf */
+VIR_FROM_LIBSSH = 66,   /* Error from libssh connection transport */
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_DOMAIN_LAST
@@ -317,6 +318,7 @@ typedef enum {
 VIR_ERR_NO_CLIENT = 96, /* Client was not found */
 VIR_ERR_AGENT_UNSYNCED = 97,/* guest agent replies with wrong id
to guest-sync command */
+VIR_ERR_LIBSSH = 98,/* error in libssh transport driver */
 } virErrorNumber;
 
 /**
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 2958308..ef17fb5 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -137,7 +137,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
   "Log Manager",
   "Xen XL Config",
 
-  "Perf",
+  "Perf", /* 65 */
+  "Libssh transport layer",
 )
 
 
@@ -1400,6 +1401,12 @@ virErrorMsg(virErrorNumber error, const char *info)
 else
 errmsg = _("guest agent replied with wrong id to guest-sync 
command: %s");
 break;
+case VIR_ERR_LIBSSH:
+if (info == NULL)
+errmsg = _("libssh transport error");
+else
+errmsg = _("libssh transport error: %s");
+break;
 }
 return errmsg;
 }
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 1/6] virNetSocket: allow to not close FD

[Pino Toscano]

Add an internal variable to mark the FD as "not owned" by the
virNetSocket, in case the internal implementation takes the actual
ownership of the descriptor; this avoids a warning when closing the
socket, as the FD would be invalid.
---
 src/rpc/virnetsocket.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 405f5ba..05f20a5 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -77,6 +77,7 @@ struct _virNetSocket {
 pid_t pid;
 int errfd;
 bool client;
+bool ownsFd;
 
 /* Event callback fields */
 virNetSocketIOFunc func;
@@ -248,6 +249,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr 
localAddr,
 sock->errfd = errfd;
 sock->pid = pid;
 sock->watch = -1;
+sock->ownsFd = true;
 
 /* Disable nagle for TCP sockets */
 if (sock->localAddr.data.sa.sa_family == AF_INET ||
@@ -1202,7 +1204,8 @@ void virNetSocketDispose(void *obj)
 virObjectUnref(sock->sshSession);
 #endif
 
-VIR_FORCE_CLOSE(sock->fd);
+if (sock->ownsFd)
+VIR_FORCE_CLOSE(sock->fd);
 VIR_FORCE_CLOSE(sock->errfd);
 
 virProcessAbort(sock->pid);
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 0/6] New libssh transport

[Pino Toscano]

Hi,

this series introduces a new libssh transport in libvirt, based on the
libssh C library.  This library supports what libssh2 does, and more:
- easier API for known_hosts handling (there's a ticket upstream to
  request extensions for it, but what is implemented now works well)
- potential GSSAPI authentication (not enabled yet because of a libssh
  bug [1])
- easier API for ssh-agent support

The implementation for the new transport is based on the libssh2 one,
hence it shares origin and style.

[1] https://red.libssh.org/issues/242

Thanks,

Changes from v2 to v3:
- split into more commits
- integrate all the issues spotted by Daniel P. Berrange and
  Peter Krempa
- restore the specified order of authentication methods (as in libssh2)
- add a related documentation fix
- minor coding fixes

Changes from v1 to v2:
- implemented keyboard interactive
- code polish, and fixes


Pino Toscano (6):
  virNetSocket: allow to not close FD
  virerror: add error for libssh transport
  libssh_transport: add new libssh-based transport
  remote: expose a new libssh transport
  spec: enable libssh transport on Fedora
  docs: fix default value for sshauth option of libssh2/libssh

 config-post.h |2 +
 configure.ac  |3 +
 docs/remote.html.in   |   39 +-
 include/libvirt/virterror.h   |2 +
 libvirt.spec.in   |   10 +
 m4/virt-libssh.m4 |   26 +
 po/POTFILES.in|1 +
 src/Makefile.am   |   21 +-
 src/libvirt_libssh.syms   |   22 +
 src/remote/remote_driver.c|   41 ++
 src/rpc/virnetclient.c|  118 
 src/rpc/virnetclient.h|   13 +
 src/rpc/virnetlibsshsession.c | 1458 +
 src/rpc/virnetlibsshsession.h |   80 +++
 src/rpc/virnetsocket.c|  184 +-
 src/rpc/virnetsocket.h|   13 +
 src/util/virerror.c   |9 +-
 17 files changed, 2024 insertions(+), 18 deletions(-)
 create mode 100644 m4/virt-libssh.m4
 create mode 100644 src/libvirt_libssh.syms
 create mode 100644 src/rpc/virnetlibsshsession.c
 create mode 100644 src/rpc/virnetlibsshsession.h

-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 01:07:25PM +0100, Richard W.M. Jones wrote:
> On Mon, Oct 03, 2016 at 04:49:47PM +0100, Daniel P. Berrange wrote:
> > --- a/src/qemu/qemu.conf
> > +++ b/src/qemu/qemu.conf
> > @@ -595,16 +595,22 @@
> >  # using this master file as image. Each UEFI firmware can,
> >  # however, have different variables store. Therefore the nvram is
> >  # a list of strings when a single item is in form of:
> > -#   ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}.
> > +#
> > +#   ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}:${ARCH}[:${FEATURE}:...].
> > +#
> > +# Current valid features include:
> > +#
> > +#   'secboot' - the firmware has secure boot enabled
> > +#
> >  # Later, when libvirt creates per domain variable store, this list is
> >  # searched for the master image. The UEFI firmware can be called
> >  # differently for different guest architectures. For instance, it's OVMF
> >  # for x86_64 and i686, but it's AAVMF for aarch64. The libvirt default
> >  # follows this scheme.
> >  #nvram = [
> > -#   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd",
> > -#   "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd",
> > -#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd"
> > +#   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64",
> > +#   
> > "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64:secboot",
> > +#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd:aarch64"
> 
> This is all good, and could remove a duplicate copy of this database
> which is currently stored in libguestfs & virt-v2v:
> 
> https://github.com/libguestfs/libguestfs/blob/master/generator/uefi.ml#L30
> 
> The flags (arch, secboot) even precisely match the ones we currently
> need to store.
> 
> Unfortunately it's a case of so near and yet so far.  You're proposing
> this essentially static and non-secret data be stored in
> /etc/libvirt/qemu.conf, which is not readable as non-root.  virt-v2v
> (which can run as non-root) would still need to store a duplicate copy
> of the data.

What does v2v need the mapping data for ?  Any use case needs to be
addressed via the APIs, not having apps poke at libvirt private
config files.

> I don't see any need for config files to default to unreadable, it's
> just security through obscurity (and not even obscure), but assuming
> that isn't going to change, please put this into a different file
> which can be read as non-root.  There is literally nothing possibly
> secret about it, it's just the location of some files.

Even if the config files were readable, this data is not going
to be present in the config file by default - it'll only be
there if the admin has needed to override libvirts builtin
default value.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] conf: add support for choosing firmware type

[Richard W.M. Jones]

On Mon, Oct 03, 2016 at 04:49:48PM +0100, Daniel P. Berrange wrote:
> Currently if you want to enable UEFI firmware for a guest
> you need to know about the hypervisor platform specific
> firmware path. This does not even work for all platforms,
> as hypervisors like VMWare don't expose UEFI as a path,
> they just have a direct config option for turning it on
> or off.
> 
> This adds ability to use the much simpler:
> 
>   
> 
> to choose the different firmware types.

Although not in the scope of this patch, virt-v2v would really like
the ESX driver in libvirt to return this data for existing guests.  I
believe there is an open RFE about that somewhere.

Patches 2 & 3 look fine to me, ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Richard W.M. Jones]

On Mon, Oct 03, 2016 at 04:49:47PM +0100, Daniel P. Berrange wrote:
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -595,16 +595,22 @@
>  # using this master file as image. Each UEFI firmware can,
>  # however, have different variables store. Therefore the nvram is
>  # a list of strings when a single item is in form of:
> -#   ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}.
> +#
> +#   ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}:${ARCH}[:${FEATURE}:...].
> +#
> +# Current valid features include:
> +#
> +#   'secboot' - the firmware has secure boot enabled
> +#
>  # Later, when libvirt creates per domain variable store, this list is
>  # searched for the master image. The UEFI firmware can be called
>  # differently for different guest architectures. For instance, it's OVMF
>  # for x86_64 and i686, but it's AAVMF for aarch64. The libvirt default
>  # follows this scheme.
>  #nvram = [
> -#   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd",
> -#   "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd",
> -#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd"
> +#   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64",
> +#   
> "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64:secboot",
> +#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd:aarch64"

This is all good, and could remove a duplicate copy of this database
which is currently stored in libguestfs & virt-v2v:

https://github.com/libguestfs/libguestfs/blob/master/generator/uefi.ml#L30

The flags (arch, secboot) even precisely match the ones we currently
need to store.

Unfortunately it's a case of so near and yet so far.  You're proposing
this essentially static and non-secret data be stored in
/etc/libvirt/qemu.conf, which is not readable as non-root.  virt-v2v
(which can run as non-root) would still need to store a duplicate copy
of the data.

I don't see any need for config files to default to unreadable, it's
just security through obscurity (and not even obscure), but assuming
that isn't going to change, please put this into a different file
which can be read as non-root.  There is literally nothing possibly
secret about it, it's just the location of some files.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] Toward a better NEWS file

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 01:53:41PM +0200, Andrea Bolognani wrote:
> Hi,
> 
> there's an idea that has been kicking around in my head for
> a while, and I'd like to share it with the list to gather
> some feedback before I forget about it :)
> 
> Right now, each entry in our NEWS file contains what is
> basically the output of
> 
>   git log \
>   --pretty=oneline \
>   vX.Y-1.0..vX.Y.0
> 
> with the commits organized somewhat arbitrarily into a bunch
> of sections with partially overlapping scopes.
> 
> I believe the current form is less than useful, as it is too
> detailed for end users and distro packagers, who only care
> about the high-level user visible changes, and not detailed
> enough for developers, who are always going to refer to the
> proper git log anyway. Moreover, we ship a ChangeLog file
> that contains very detailed information already.
> 
> Ideally, the NEWS file would contain a summary of notable
> changes (new APIs, significantly improved features, etc.)
> laid out in an easy to digest fashion, such that people who
> are not knee-deep into libvirt development can grasp them
> and hopefully get excited about upgrading to our latest and
> greatest release :)
> 
> Of course, it would take an insane amount of time for any
> single one of us to turn the git log into such a document,
> and the result would still be sub-par because we simply
> can't expect anyone to have full insight in every single
> change to the project.
> 
> My solution for this is to ask the people with the most
> knowledge of the changes - the authors themselves!
> 
> The workflow I'm envisioning would look like this:
> 
>   * DV, at the same time as he announces that libvirt has
> entered freeze, will put out a Call for NEWS and ask
> people who have contributed code to the upcoming release
> to post a summary of their changes
>
>   * the authors will go over
> 
>   git log \
>   --author=$(git config user.email) \
>   vX.Y-1.0..master
> 
> and come up with a short (one-three sentences) summary
> for each of the changes, if they are notable. Commits
> that are part of a larger series would not be described
> on their own: a short summary of the series would be
> used instead, akin to the one you would put in your
> cover letter.
> 
> To give a practical example: I've mostly been busy with
> reviews this cycle, but if I were to go over my commits
> since v2.3.0 right now I would write something like
> 
>   * Bug fix: don't restart libvirt-guests.service when
> libvirtd.service is restarted
> 
> for commit 2273bbd, and omit both 61e1014 and a0da413 as
> they're neither notable enough on their own, nor part of
> a larger series: we'll always have a "various bug fixes
> and improvements" bullet point in a NEWS file entry to
> take care of that kind of small cleanups and improvements.
> 
>   * the authors would post the resulting summaries to the
> list. We could simply post them as regular patches to
> docs/news.html.in (potentially without requiring review
> before pushing them), or post them as plain text and have
> someone collect them and prepare a single commit
> 
>   * DV will tag the release and push the tarballs, and
> everyone will be able to enjoy the NEWS file :)
> 
> Some light editoral work might be needed throughout the
> process, eg. fix typos or post one or two reminders during
> the freeze: I volunteer to take such tasks upon myself.
> 
> I'm looking forward to feedback about this idea, especially
> from people who might be part of any community where anything
> like this is already happening.

Why don't we simply have a NEWS file in GIT, and require that
non-trivial commits or patch series include an update to NEWS,
so the NEWS file gets populated at time the feature/bug fix
gets merged.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC] Toward a better NEWS file

[Andrea Bolognani]

Hi,

there's an idea that has been kicking around in my head for
a while, and I'd like to share it with the list to gather
some feedback before I forget about it :)

Right now, each entry in our NEWS file contains what is
basically the output of

  git log \
  --pretty=oneline \
  vX.Y-1.0..vX.Y.0

with the commits organized somewhat arbitrarily into a bunch
of sections with partially overlapping scopes.

I believe the current form is less than useful, as it is too
detailed for end users and distro packagers, who only care
about the high-level user visible changes, and not detailed
enough for developers, who are always going to refer to the
proper git log anyway. Moreover, we ship a ChangeLog file
that contains very detailed information already.

Ideally, the NEWS file would contain a summary of notable
changes (new APIs, significantly improved features, etc.)
laid out in an easy to digest fashion, such that people who
are not knee-deep into libvirt development can grasp them
and hopefully get excited about upgrading to our latest and
greatest release :)

Of course, it would take an insane amount of time for any
single one of us to turn the git log into such a document,
and the result would still be sub-par because we simply
can't expect anyone to have full insight in every single
change to the project.

My solution for this is to ask the people with the most
knowledge of the changes - the authors themselves!

The workflow I'm envisioning would look like this:

  * DV, at the same time as he announces that libvirt has
entered freeze, will put out a Call for NEWS and ask
people who have contributed code to the upcoming release
to post a summary of their changes

  * the authors will go over

  git log \
  --author=$(git config user.email) \
  vX.Y-1.0..master

and come up with a short (one-three sentences) summary
for each of the changes, if they are notable. Commits
that are part of a larger series would not be described
on their own: a short summary of the series would be
used instead, akin to the one you would put in your
cover letter.

To give a practical example: I've mostly been busy with
reviews this cycle, but if I were to go over my commits
since v2.3.0 right now I would write something like

  * Bug fix: don't restart libvirt-guests.service when
libvirtd.service is restarted

for commit 2273bbd, and omit both 61e1014 and a0da413 as
they're neither notable enough on their own, nor part of
a larger series: we'll always have a "various bug fixes
and improvements" bullet point in a NEWS file entry to
take care of that kind of small cleanups and improvements.

  * the authors would post the resulting summaries to the
list. We could simply post them as regular patches to
docs/news.html.in (potentially without requiring review
before pushing them), or post them as plain text and have
someone collect them and prepare a single commit

  * DV will tag the release and push the tarballs, and
everyone will be able to enjoy the NEWS file :)

Some light editoral work might be needed throughout the
process, eg. fix typos or post one or two reminders during
the freeze: I volunteer to take such tasks upon myself.

I'm looking forward to feedback about this idea, especially
from people who might be part of any community where anything
like this is already happening.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] xl: don't output (null) target in domxml-to-native

[Peter Krempa]

On Wed, Oct 19, 2016 at 13:20:09 +0200, Peter Krempa wrote:
> On Wed, Oct 19, 2016 at 12:10:00 +0200, Cédric Bosdonnat wrote:
> > When converting a domain xml containing a CDROM device without
> > any attached source, don't add a target=(null) to the libxl config
> > disk definition: xen doesn't like it at all and would fail to start
> > the domain.
> > ---
> >  src/xenconfig/xen_xl.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> > index a06983e..db8cbf1 100644
> > --- a/src/xenconfig/xen_xl.c
> > +++ b/src/xenconfig/xen_xl.c
> > @@ -1068,7 +1068,7 @@ xenFormatXLDisk(virConfValuePtr list, 
> > virDomainDiskDefPtr disk)
> >  
> >  /* devtype */
> >  if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> > -virBufferAddLit(, "devtype=cdrom,");
> > +virBufferAddLit(, "devtype=cdrom");
> 
> This ... 
> 
> >  
> >  /*
> >   * target
> > @@ -1081,7 +1081,9 @@ xenFormatXLDisk(virConfValuePtr list, 
> > virDomainDiskDefPtr disk)
> >  if (xenFormatXLDiskSrc(disk->src, ) < 0)
> 
> ... would drop the comma leading the disk source in cases when the CDROM
> is actually full.

Disregard this. I did not notice that 'target' is used to transfer the
string.

> 
> >  goto cleanup;
> >  
> > -virBufferAsprintf(, "target=%s", target);
> > +if (target) {
> > +virBufferAsprintf(, ",target=%s", target);
> > +}
> 
> This fails syntax-check.

ACK if you make sure that you run syntax-check before pushing.


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] xl: don't output (null) target in domxml-to-native

[Peter Krempa]

On Wed, Oct 19, 2016 at 12:10:00 +0200, Cédric Bosdonnat wrote:
> When converting a domain xml containing a CDROM device without
> any attached source, don't add a target=(null) to the libxl config
> disk definition: xen doesn't like it at all and would fail to start
> the domain.
> ---
>  src/xenconfig/xen_xl.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index a06983e..db8cbf1 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -1068,7 +1068,7 @@ xenFormatXLDisk(virConfValuePtr list, 
> virDomainDiskDefPtr disk)
>  
>  /* devtype */
>  if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> -virBufferAddLit(, "devtype=cdrom,");
> +virBufferAddLit(, "devtype=cdrom");

This ... 

>  
>  /*
>   * target
> @@ -1081,7 +1081,9 @@ xenFormatXLDisk(virConfValuePtr list, 
> virDomainDiskDefPtr disk)
>  if (xenFormatXLDiskSrc(disk->src, ) < 0)

... would drop the comma leading the disk source in cases when the CDROM
is actually full.

>  goto cleanup;
>  
> -virBufferAsprintf(, "target=%s", target);
> +if (target) {
> +virBufferAsprintf(, ",target=%s", target);
> +}

This fails syntax-check.


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

[Pino Toscano]

On Tuesday, 18 October 2016 15:49:56 CEST Daniel P. Berrange wrote:
> On Tue, Oct 18, 2016 at 04:46:53PM +0200, Pino Toscano wrote:
> > On Tuesday, 18 October 2016 14:19:41 CEST Daniel P. Berrange wrote:
> > > On Mon, Oct 17, 2016 at 04:24:53PM +0200, Pino Toscano wrote:
> > > > Implement a new libssh transport, which uses libssh to communicate with
> > > > remote hosts, and use it in virNetSockets.
> > > > 
> > > > This new transport supports all the common ssh authentication methods,
> > > > making use of libvirt's auth callbacks for interaction with the user.
> > > > 
> > > > Most of the functionalities and implementation are based on the libssh2
> > > > transport.
> > > > ---
> > > >  config-post.h |2 +
> > > >  configure.ac  |3 +
> > > >  include/libvirt/virterror.h   |2 +
> > > >  m4/virt-libssh.m4 |   26 +
> > > >  src/Makefile.am   |   21 +-
> > > >  src/libvirt_libssh.syms   |   22 +
> > > >  src/remote/remote_driver.c|   41 ++
> > > >  src/rpc/virnetclient.c|  123 
> > > >  src/rpc/virnetclient.h|   13 +
> > > >  src/rpc/virnetlibsshsession.c | 1424 
> > > > +
> > > >  src/rpc/virnetlibsshsession.h |   80 +++
> > > >  src/rpc/virnetsocket.c|  179 ++
> > > >  src/rpc/virnetsocket.h|   13 +
> > > >  src/util/virerror.c   |9 +-
> > > >  14 files changed, 1955 insertions(+), 3 deletions(-)
> > > >  create mode 100644 m4/virt-libssh.m4
> > > >  create mode 100644 src/libvirt_libssh.syms
> > > >  create mode 100644 src/rpc/virnetlibsshsession.c
> > > >  create mode 100644 src/rpc/virnetlibsshsession.h
> > > 
> > > libvirt.spec.in and mingw-libvirt.spec.in need updating too, as well
> > > as docs/remote.html.in
> > 
> > OK for libvirt.spec.in and docs/remote.html.in, but libssh for mingw is
> > not available in Fedora.
> 
> Is it possible to build libssh for mingw ?  If so we'll need to look at
> getting that into Fedora.

I don't know -- I can request that to its maintainer once the libssh
transport is in (so there is an actual use case for adding mingw
support).

At least, libssh is supposed to build and work on Windows as well.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

[Pino Toscano]

On Wednesday, 19 October 2016 08:33:27 CEST Peter Krempa wrote:
> > > > +memset(_passphrase, 0, sizeof(virConnectCredential));
> > > > +retr_passphrase.type = virCredTypeForPrompt(sess->cred, echo);
> > > > +retr_passphrase.prompt = virBufferCurrentContent();
> > > 
> > > This shouldn't really be used. Please get the content into a variable.
> > 
> > Not a problem to change this, but could you please explain a bit more
> > on the reasons? (So I can avoid doing the same again.)
> 
> Basically any call any function that uses the buffer invalidates the
> pointer returned by virBufferCurrentContent (or should be treated as
> such) which is prone to bugs when later modifying the code.
> 
> Most of the uses of the function are in special cases (or should be
> removed).

I see, thanks for the explanation.

> > > > +int
> > > > +virNetLibsshSessionConnect(virNetLibsshSessionPtr sess,
> > > > +   int sock)
> > > > +{
> > > > +int ret;
> > > > +const char *errmsg;
> > > > +
> > > > +VIR_DEBUG("sess=%p, sock=%d", sess, sock);
> > > > +
> > > > +if (!sess || sess->state != VIR_NET_LIBSSH_STATE_NEW) {
> > > > +virReportError(VIR_ERR_LIBSSH, "%s",
> > > > +   _("Invalid virNetLibsshSessionPtr"));
> > > > +return -1;
> > > > +}
> > > > +
> > > > +virObjectLock(sess);
> > > > +
> > > > +/* check if configuration is valid */
> > > > +if ((ret = virNetLibsshValidateConfig(sess)) < 0)
> > > > +goto error;
> > > > +
> > > > +/* read ~/.ssh/config */
> > > > +if ((ret = ssh_options_parse_config(sess->session, NULL)) < 0)
> > > > +goto error;
> > > > +
> > > > +/* set the socket FD for the libssh session */
> > > > +if ((ret = ssh_options_set(sess->session, SSH_OPTIONS_FD, )) 
> > > > < 0)
> > > 
> > > Is this guaranteed to copy the socket number at call time? Otherwise
> > > (similarly to the ones above will not work reliably).
> > 
> > I don't understand this sentence (it seems truncated), can you please
> > rephrase it?
> 
> Sorry I probably got sidetracked and did not finish my thought.
> 
> My question was whether ssh_options_set accesses the pointer to 'sock'
> right away and copies it. You are passing a pointer to a stack allocated
> variable which will get out of scope later, thus the pointer should not
> be accessed after the call to ssh_options_set returns.

libssh indeed copies the values passed to ssh_options_set, so the two
calls you mentioned (for SSH_OPTIONS_FD and SSH_OPTIONS_PORT) are safe.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] xl: don't output (null) target in domxml-to-native

[Cédric Bosdonnat]

When converting a domain xml containing a CDROM device without
any attached source, don't add a target=(null) to the libxl config
disk definition: xen doesn't like it at all and would fail to start
the domain.
---
 src/xenconfig/xen_xl.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index a06983e..db8cbf1 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -1068,7 +1068,7 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr 
disk)
 
 /* devtype */
 if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
-virBufferAddLit(, "devtype=cdrom,");
+virBufferAddLit(, "devtype=cdrom");
 
 /*
  * target
@@ -1081,7 +1081,9 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr 
disk)
 if (xenFormatXLDiskSrc(disk->src, ) < 0)
 goto cleanup;
 
-virBufferAsprintf(, "target=%s", target);
+if (target) {
+virBufferAsprintf(, ",target=%s", target);
+}
 
 if (virBufferCheckError() < 0)
 goto cleanup;
-- 
2.10.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] vz: set localhost as vnc address

[Nikolay Shirokovskiy]

On 18.10.2016 19:19, Mikhail Feoktistov wrote:
> We should set localhost as vnc address in case of empty string.
> Because Virtuozzo sets 0.0.0.0 as default vnc address.
> ---
>  src/vz/vz_sdk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index f2a5c96..7235172 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -2967,7 +2967,7 @@ static int prlsdkApplyGraphicsParams(PRL_HANDLE sdkdom,
>  
>  glisten = virDomainGraphicsGetListen(gr, 0);
>  pret = PrlVmCfg_SetVNCHostName(sdkdom, glisten && glisten->address ?
> -   glisten->address : "");
> +   glisten->address : "127.0.0.1");
>  prlsdkCheckRetGoto(pret, cleanup);
>  
>  ret = 0;
> 

ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 00/25] Use more PCIe less legacy PCI

[Andrea Bolognani]

On Fri, 2016-10-14 at 15:53 -0400, Laine Stump wrote:
> The latest version of patches to auto-assign appropriate devices in
> PCIe-capable domains to PCIe slots rather than legacy PCI, and to also
> auto-add pcie-root-ports as necessary to make this easier.
> 
> I *think* I've dealt with all of Andrea's great critiques of V3,
> either in my email responses or in these revised patches. Aside from
> fixing typos and formatting problems, I also moved several "little
> cosmetic fixes" out of the bigger functionality-changing patches, and
> also split out the most confusing part of patch 20 (previously patch
> 15 - the one that auto-adds pcie-root-port and dmi-to-pci-bridge
> controllers) into its own patch (doubly useful since the value of that
> part of the patch is dubious - it is now the [RFC] patch 21)
> 
> As many of the simple costmetic patches as possible were pushed up to
> the front of the series (if they weren't already there). If those are
> ACKed on this round but others aren't, I'll push them immediately so I
> don't need to resubmit the whole series again.

I think you can go ahead and push patches 2-11 now - of
course after taking care of the trivial stuff I pointed out
during this last round of reviews.

Patch 1 doesn't make sense until patch 12, so please hold off
pushing that one for the time being.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu_driver: unlink new domain cfg file when rollbak

[Chen Hanxiao]

At 2016-10-14 17:35:26, "Chen Hanxiao"  wrote:
>From: Chen Hanxiao 
>
>If we failed to unlink old dom cfg file, we goto rollback.
>But inside rollback, we fogot to unlink the new dom cfg file.
>This patch fixes this issue.
>

ping?

Regards,
- Chen

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] cpu_conf: add comments about sockets in cpu_conf

[Chen Hanxiao]

At 2016-10-18 21:59:44, "Peter Krempa"  wrote:
>On Tue, Oct 18, 2016 at 16:18:10 +0800, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
>> 
>> 'sockets' in output of `virsh capabilities' means
>
>You are talking here about virsh capabilities, while the code is about
>the cpu definition in the domain configuration.
>
>> the sockets per NUMA node,
>> which is a special case.
>
>Indeed it is a special case in the capabilities XML. So why are you
>changing the code relevant to domain definition, where the sockets
>count is actually the total sockets count?
>
>> discuss in:
>> https://www.redhat.com/archives/libvir-list/2012-March/msg01123.html
>
>https://www.redhat.com/archives/libvir-list/2012-March/msg01133.html
>describes why sockets per NUMA node is not really what we want.

Thanks for the clarification, I missed that thread.

Regards,
- Chen

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] make virDomainQemuMonitorCommand work in any libvirt state

[Nikolay Shirokovskiy]

On 19.10.2016 10:37, Daniel P. Berrange wrote:
> On Wed, Oct 19, 2016 at 09:50:37AM +0800, Michal Privoznik wrote:
>> On 17.10.2016 20:46, Nikolay Shirokovskiy wrote:
>>> Hi, all.
>>>
>>> We would like to use virDomainQemuMonitorCommand to query qemu 
>>> independently of
>>> libvirt state. Currenly it is not possible. This API call takes job 
>>> condition
>>> just like any other call and thus is unavailable on any lengthy(or stucked)
>>> synchronous job.
>>>
>>> I've already posted this question in list, just failed to find the 
>>> reference.
>>> Somebody suggested to use proxy (and even an implementation) in between qemu
>>> and libvirt that can inject commands to qemu and filter replies. It is not
>>> really convinient. This way test setups will be different from production 
>>> and
>>> we can not investigate problems in production environment.
>>>
>>> I'd like to drop acquiring job condition in the call as this function does 
>>> not
>>> deal with libvirt state (except for the taint but is is ok, we will not mess
>>> things up here). But this is not enough, we need to make qemu monitor deal 
>>> with
>>> many qemu commands simultaneously. Looks like it is quite a big change for
>>> test/debug case. But I guess eventually normal user cases can get benefits 
>>> too
>>> from this monitor changes. For example all query API calls that query qemu
>>> directly can be changed to not to wait for some synchronous job
>>> finishing.(qemuDomainGetBlockJobInfo for example).
>>
>> IIRC the last time I looked into this the problem was not on libvirt
>> side. QEMU's monitor was unable to process multiple commands at once.
>> But maybe that's no longer the case, I don't know.
> 
> That is still the case. If you send a command while something else
> is running QEMU will not process it until the previous command is
> completed.

Ok. Then now is not appropriate time to address the problem in libvirt.

Denis mentioned another case when libvirt is too restrictive on
parallel execution. API calls to agent and qemu are serialized
while at the first glance they not have to. We would like to 
introduce different job condition for agent API calls. And if
agent can process multiple commands at once we can change agent
monitor to issue multiple commands too.

> 
> There was a proposal last week to address this in QEMU, but it is
> not going to be fully fixed anytime soon.
> 
>> However, what I think we should do is to turn our jobs into sort of RW
>> locks. That is - we could allow multiple QUERY jobs to happen
>> simultaneously and leave MODIFY jobs to be exclusive. I think dropping
>> BeginJob() from an API is a no go as it will definitely bite us in the
>> future.
>>
>> Unfortunately, I have no idea what my suggestion would look like in
>> terms of the code. How difficult it would be to implement it (and
>> whether monitor code is prepared for that).
> 

Nikolay

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 11/25] [ACKED] qemu: replace calls to virDomainPCIAddressReserveNext*() with static function

[Andrea Bolognani]

On Fri, 2016-10-14 at 15:54 -0400, Laine Stump wrote:
> An upcoming commit will remove the "flag" argument from all the calls
> to reserve the next available address|slot, but I don't want to change
> the arguments in the hypervisor-agnostic
> virDomainPCIAddressReserveNext*() functions, so this patch places a
> simple qemu-specific wrapper around those functions - the new
> functions don't take a flags arg, but grab it from the device's
> info->pciConnectFlags.
> ---
> Change: realigned/reformatted
> 
>  src/qemu/qemu_domain_address.c | 96 
>++
>  1 file changed, 59 insertions(+), 37 deletions(-)

[...]
> @@ -691,7 +712,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>  
>  if (virDomainPCIAddressSlotInUse(addrs, _addr)) {
>  if (qemuDeviceVideoUsable) {
> -if (virDomainPCIAddressReserveNextSlot(addrs,
> +if (qemuDomainPCIAddressReserveNextSlot(addrs,
> 
>>info,
> flags) < 0)

Alignment's still off here.

ACK with that fixed.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] util: Add function to check if string contains some chars

[Sławek Kapłoński]

Hello,


-- 
Best regards / Pozdrawiam
Sławek Kapłoński
sla...@kaplonski.pl

On Wed, 19 Oct 2016, Michal Privoznik wrote:

> On 19.10.2016 03:55, Sławek Kapłoński wrote:
> > Tue, 18 Oct 2016, Michal Privoznik wrote:
> >> > On 14.10.2016 04:53, Sławek Kapłoński wrote:
> >>> > > This new function can be used to check if e.g. name of XML node
> >>> > > don't contains forbidden chars like "/" or new-line.
> >>> > > ---
> >>> > >  src/conf/network_conf.c  | 2 +-
> >>> > >  src/libvirt_private.syms | 1 +
> >>> > >  src/util/virstring.c | 9 +
> >>> > >  src/util/virstring.h | 1 +
> >>> > >  4 files changed, 12 insertions(+), 1 deletion(-)
> >>> > > 
> >>> > > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> >>> > > index aa39776..949d138 100644
> >>> > > --- a/src/conf/network_conf.c
> >>> > > +++ b/src/conf/network_conf.c
> >>> > > @@ -2117,7 +2117,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> >>> > >  goto error;
> >>> > >  }
> >>> > >  
> >>> > > -if (strchr(def->name, '/')) {
> >>> > > +if (virStringHasChars(def->name, "/")) {
> >>> > >  virReportError(VIR_ERR_XML_ERROR,
> >>> > > _("name %s cannot contain '/'"), def->name);
> >> > 
> >> > Usually, in one patch we introduce a function and then in a subsequent
> >> > one we switch the rest of the code into using it. But okay, I can live
> >> > with this too.
> >> > 
> >>> > >  goto error;
> >>> > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> >>> > > index 55b6a24..6f41234 100644
> >>> > > --- a/src/libvirt_private.syms
> >>> > > +++ b/src/libvirt_private.syms
> >>> > > @@ -2435,6 +2435,7 @@ virStringEncodeBase64;
> >>> > >  virStringFreeList;
> >>> > >  virStringFreeListCount;
> >>> > >  virStringGetFirstWithPrefix;
> >>> > > +virStringHasChars;
> >>> > >  virStringHasControlChars;
> >>> > >  virStringIsEmpty;
> >>> > >  virStringIsPrintable;
> >>> > > diff --git a/src/util/virstring.c b/src/util/virstring.c
> >>> > > index 4a70620..7799d87 100644
> >>> > > --- a/src/util/virstring.c
> >>> > > +++ b/src/util/virstring.c
> >>> > > @@ -975,6 +975,15 @@ virStringStripIPv6Brackets(char *str)
> >>> > >  }
> >>> > >  
> >>> > >  
> >>> > > +bool
> >>> > > +virStringHasChars(const char *str, const char *chars)
> >>> > > +{
> >>> > > +if (strpbrk(str, chars))
> >>> > > +return true;
> >>> > > +return false;
> >>> > > +}
> >> > 
> >> > This, however, makes not much sense. I mean, this function has no added
> >> > value to pain strpbrk(). What I suggested in one of previous reviews was
> >> > that this function should report an error too. In that case, it will
> >> > immediately have added value and thus it will be handy to use it.
> >> > Perhaps you are afraid that error message might change in some cases?
> >> > That's okay, we don't make any promises about error messages.
> >> > 
> > I agree that in fact it don't have too much sense but I'm not sure that
> > it would be good to report error here. First of all, it could be that in
> > some cases it could be used to check if function have required chars so
> > there is no easy way to check which result is error in fact.
> 
> Well, even if we did want to check for that, strpbrk() is no help there.
> I mean, you cannot use it to check if a string contains only allowed
> characters and nothing more.
> 
> > Second thing (maybe here I'm wrong) places where I wanted to use this
> > function are reporting error VIR_ERR_XML_ERROR and I'm not sure it is
> > good place to report such error in "string lib".
> 
> That's why I initially suggested for this function to be in virxml.c
> (and thus have a slightly different name to reflect the submodule it is in).
> 
> > So maybe better solution would be just use strbprk (or strchr) in
> > src/network/bridge_driver.c to check if name contains \n char and not
> > introduce any new function to such check? What You think about that?
> 
> Well, then again - I don't think we should limit ourselves to network
> driver really. Other drivers suffer from the same problem, don't they? I
> mean, sure - we can just use strpbrk() here and copy it all over the
> place to different drivers, but I think having a small function just for
> that would be more convenient.
> Moreover, we already have some small, one purpose functions in virxml,
> for instance:  virXMLPickShellSafeComment, virXMLSaveFile,
> virXMLPropString, and others.

Ok, that makes sense. I will add this function to virxml.c then and will
rename it. Then it should be fine to report error message also in such
function. Once again, thx for help with that.

> 
> Michal


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] make virDomainQemuMonitorCommand work in any libvirt state

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 09:50:37AM +0800, Michal Privoznik wrote:
> On 17.10.2016 20:46, Nikolay Shirokovskiy wrote:
> > Hi, all.
> > 
> > We would like to use virDomainQemuMonitorCommand to query qemu 
> > independently of
> > libvirt state. Currenly it is not possible. This API call takes job 
> > condition
> > just like any other call and thus is unavailable on any lengthy(or stucked)
> > synchronous job.
> > 
> > I've already posted this question in list, just failed to find the 
> > reference.
> > Somebody suggested to use proxy (and even an implementation) in between qemu
> > and libvirt that can inject commands to qemu and filter replies. It is not
> > really convinient. This way test setups will be different from production 
> > and
> > we can not investigate problems in production environment.
> > 
> > I'd like to drop acquiring job condition in the call as this function does 
> > not
> > deal with libvirt state (except for the taint but is is ok, we will not mess
> > things up here). But this is not enough, we need to make qemu monitor deal 
> > with
> > many qemu commands simultaneously. Looks like it is quite a big change for
> > test/debug case. But I guess eventually normal user cases can get benefits 
> > too
> > from this monitor changes. For example all query API calls that query qemu
> > directly can be changed to not to wait for some synchronous job
> > finishing.(qemuDomainGetBlockJobInfo for example).
> 
> IIRC the last time I looked into this the problem was not on libvirt
> side. QEMU's monitor was unable to process multiple commands at once.
> But maybe that's no longer the case, I don't know.

That is still the case. If you send a command while something else
is running QEMU will not process it until the previous command is
completed.

There was a proposal last week to address this in QEMU, but it is
not going to be fully fixed anytime soon.

> However, what I think we should do is to turn our jobs into sort of RW
> locks. That is - we could allow multiple QUERY jobs to happen
> simultaneously and leave MODIFY jobs to be exclusive. I think dropping
> BeginJob() from an API is a no go as it will definitely bite us in the
> future.
> 
> Unfortunately, I have no idea what my suggestion would look like in
> terms of the code. How difficult it would be to implement it (and
> whether monitor code is prepared for that).

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] How libvirt address qemu command line args

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 10:17:21AM +0800, Michal Privoznik wrote:
> On 18.10.2016 14:59, zhun...@gmail.com wrote:
> > Now I want to add some args about TPM  to domain's XML,so I can start a 
> > domain by virt-manager or other virsh command,and then ,I would like to use 
> > sVIrt security context to label vTPM and correspondingVM,But I do not know 
> > how to get these XML  args in libvirt.
> > the key problem is that how can i get and recognize these args!!!
> > related XML content :
> 
> Usually, grepping the code for cmd name <-> XML element/attribute
> translation is sufficient (esp. if you grep tests/)
> 
> > 
> > 
> > 
> 
> Firstly, this is obsolete in favour of "-machine accel=kvm". In any
> case,  will do the trick (libvirt will use whatever
> is supported by qemu binary in your system).
> 
> > 
> >  > value='file=/root/nvram_2.0-jin.qcow2,if=none,id=nvram0-0-0,format=qcow2'/>
> 
> Okay, this is not supported by libvirt yet. We don't really have a way
> how to specify NVRAM in anything other than a raw file. BTW: isn't qcow
> too big gun for NVRAM? I mean, NVRAM has a fixed size of what ~190 KB?
> QCOW header is about the same size.
> 
> > 
> > 
> > 
> > 
> 
> I'm not sure there's a way how to put startup=clean on the cmd line. I'm
> not even sure what it does.
> And I have not idea what libtpms is either :-)
> 
> > 
> > 
> >   
> >

On top of all that - QEMU is likely to fail to start since libvirt by
default runs it as qemu:qemu user/group, and so it won't have permission
to read any of the files in /root. If you have selinux/apparmour that
will also block permission.

This is an example of why usage of qemu:commandline is discouraged - it
will always have problems with permissions if you pass files using it.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] make virDomainQemuMonitorCommand work in any libvirt state

[Denis V. Lunev]

On 10/19/2016 04:50 AM, Michal Privoznik wrote:
> On 17.10.2016 20:46, Nikolay Shirokovskiy wrote:
>> Hi, all.
>>
>> We would like to use virDomainQemuMonitorCommand to query qemu independently 
>> of
>> libvirt state. Currenly it is not possible. This API call takes job condition
>> just like any other call and thus is unavailable on any lengthy(or stucked)
>> synchronous job.
>>
>> I've already posted this question in list, just failed to find the reference.
>> Somebody suggested to use proxy (and even an implementation) in between qemu
>> and libvirt that can inject commands to qemu and filter replies. It is not
>> really convinient. This way test setups will be different from production and
>> we can not investigate problems in production environment.
>>
>> I'd like to drop acquiring job condition in the call as this function does 
>> not
>> deal with libvirt state (except for the taint but is is ok, we will not mess
>> things up here). But this is not enough, we need to make qemu monitor deal 
>> with
>> many qemu commands simultaneously. Looks like it is quite a big change for
>> test/debug case. But I guess eventually normal user cases can get benefits 
>> too
>> from this monitor changes. For example all query API calls that query qemu
>> directly can be changed to not to wait for some synchronous job
>> finishing.(qemuDomainGetBlockJobInfo for example).
> IIRC the last time I looked into this the problem was not on libvirt
> side. QEMU's monitor was unable to process multiple commands at once.
> But maybe that's no longer the case, I don't know.
>
> However, what I think we should do is to turn our jobs into sort of RW
> locks. That is - we could allow multiple QUERY jobs to happen
> simultaneously and leave MODIFY jobs to be exclusive. I think dropping
> BeginJob() from an API is a no go as it will definitely bite us in the
> future.
>
> Unfortunately, I have no idea what my suggestion would look like in
> terms of the code. How difficult it would be to implement it (and
> whether monitor code is prepared for that).
>
> Michal
The problem may be is that the lock is taken in a lot of places, f.e.
when the
command to perform fsfreeze is sent to the guest agent (monitor is not
touched) and the guest deadlocks inside VSS. In this case I have to use
gdb to diagnose entire chain including serial ports state.

This is the problem which should be addressed. If monitor is unable to
answer. OK. The hang inside QEMU and should be fixed. But right now
the bottleneck is inside libvirt and it is really painful.

Den

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

[Peter Krempa]

On Tue, Oct 18, 2016 at 16:42:26 +0200, Pino Toscano wrote:
> On Tuesday, 18 October 2016 15:15:07 CEST Peter Krempa wrote:
> > On Mon, Oct 17, 2016 at 16:24:53 +0200, Pino Toscano wrote:
> > > Implement a new libssh transport, which uses libssh to communicate with
> > > remote hosts, and use it in virNetSockets.
> > > 
> > > This new transport supports all the common ssh authentication methods,
> > > making use of libvirt's auth callbacks for interaction with the user.
> > > 
> > > Most of the functionalities and implementation are based on the libssh2
> > > transport.
> > > ---
> > >  config-post.h |2 +
> > >  configure.ac  |3 +
> > >  include/libvirt/virterror.h   |2 +
> > >  m4/virt-libssh.m4 |   26 +
> > >  src/Makefile.am   |   21 +-
> > >  src/libvirt_libssh.syms   |   22 +
> > >  src/remote/remote_driver.c|   41 ++
> > >  src/rpc/virnetclient.c|  123 
> > >  src/rpc/virnetclient.h|   13 +
> > >  src/rpc/virnetlibsshsession.c | 1424 
> > > +
> > >  src/rpc/virnetlibsshsession.h |   80 +++
> > >  src/rpc/virnetsocket.c|  179 ++
> > >  src/rpc/virnetsocket.h|   13 +
> > >  src/util/virerror.c   |9 +-
> > >  14 files changed, 1955 insertions(+), 3 deletions(-)
> > >  create mode 100644 m4/virt-libssh.m4
> > >  create mode 100644 src/libvirt_libssh.syms
> > >  create mode 100644 src/rpc/virnetlibsshsession.c
> > >  create mode 100644 src/rpc/virnetlibsshsession.h
> > 
> > Is it possible to split out the src/rpc/virnetlibsshsession.(ch) changes
> > (plus the ones necessary to compile it) from adding all the bits that
> > actually make use of it? This patch is very big.
> 
> Yes, it is -- I was not sure how much should the changes be split,
> especially in cases like this when adding a new "thing" which is used
> only in a single place later on.
> 
> Just to be sure, a reasonable split for this patch would be:
> 1) add libssh bits in virerror
> 2) add virnetlibsshsession.(ch) + autofoo stuff for libssh
> 3) add glue code in virNetSocket + virNetClient
> or were you thinking about something else? (no problems on my side)

No, this is exactly the split I was thinking about.

> > > diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> > > index 361dc1a..b296aac 100644
> > > --- a/src/rpc/virnetclient.c
> > > +++ b/src/rpc/virnetclient.c
> > > @@ -505,6 +505,129 @@ virNetClientPtr virNetClientNewLibSSH2(const char 
> > > *host,
> > >  }
> > >  #undef DEFAULT_VALUE
> > >  
> > > +#define DEFAULT_VALUE(VAR, VAL) \
> > > +if (!VAR)   \
> > > +VAR = VAL;
> > > +virNetClientPtr virNetClientNewLibssh(const char *host,
> > > +  const char *port,
> > > +  int family,
> > > +  const char *username,
> > > +  const char *privkeyPath,
> > > +  const char *knownHostsPath,
> > > +  const char *knownHostsVerify,
> > > +  const char *authMethods,
> > > +  const char *netcatPath,
> > > +  const char *socketPath,
> > > +  virConnectAuthPtr authPtr,
> > > +  virURIPtr uri)
> > > +{
> > > +virNetSocketPtr sock = NULL;
> > > +virNetClientPtr ret = NULL;
> > > +
> > > +virBuffer buf = VIR_BUFFER_INITIALIZER;
> > > +char *nc = NULL;
> > > +char *command = NULL;
> > > +
> > > +char *homedir = virGetUserDirectory();
> > > +char *confdir = virGetUserConfigDirectory();
> > > +char *knownhosts = NULL;
> > > +char *privkey = NULL;
> > > +
> > > +/* Use default paths for known hosts an public keys if not provided 
> > > */
> > 
> > So is libssh able to handle e.g. ECDSA keys in known hosts? Libssh2 was
> > not and truncated the known hosts file which was not acceptable.
> 
> Yes, it is. For example in my tests I'm passing
>   known_hosts=$HOME/.ssh/known_hosts
> as additional query item to the qemu+libssh URLs, and it works well.

Cool!

> > > +if (confdir) {
> > > +if (!knownHostsPath) {
> > > +if (virFileExists(confdir)) {
> > > +virBufferAsprintf(, "%s/known_hosts", confdir);
> > > +if (!(knownhosts = virBufferContentAndReset()))
> > 
> > Use virAsprintf instead of the two lines above.
> 
> OK.
> 
> Small side note: all the glue code in virNetSocket, virNetClient and
> remote_driver.c was basically a copy from the libssh2
> implementation, so all these fixes should be done there too.

Hmm, I'm wondering whether that code was refactored or was wrong from
the beginning :).

Anyways I'll probably visit it and clean it up.

> > > + 

Re: [libvirt] [PATCH v2 0/3] fix hot(un)plug of chardev devices with TLS encryption

[Pavel Hrdina]

On Tue, Oct 18, 2016 at 03:02:10PM -0400, John Ferlan wrote:
> 
> 
> On 10/18/2016 11:04 AM, Pavel Hrdina wrote:
> > Pavel Hrdina (3):
> >   qemu_alias: introduce qemuAliasChardevFromDevAlias helper
> >   qemu_command: create prefixed alias to separate variable
> >   qemu: always generate the same alias for tls-creds-x509 object
> > 
> >  src/qemu/qemu_alias.c  | 16 +
> >  src/qemu/qemu_alias.h  |  3 ++
> >  src/qemu/qemu_command.c| 41 
> > --
> >  src/qemu/qemu_hotplug.c| 23 +++-
> >  ...xml2argv-serial-tcp-tlsx509-chardev-verify.args |  4 +--
> >  .../qemuxml2argv-serial-tcp-tlsx509-chardev.args   |  4 +--
> >  6 files changed, 60 insertions(+), 31 deletions(-)
> > 
> 
> 
> ACK series

Thanks

Pavel


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list