Re: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev

2016-10-17 Thread Pavel Hrdina 
On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
> Add an optional "tls='yes|no'" attribute for a TCP chardev for the
> express purpose to disable setting up TLS for the specific chardev in
> the event the qemu.conf settings have enabled hypervisor wide TLS for
> serial TCP chardevs.
> 
> Signed-off-by: John Ferlan 
> ---
>  docs/formatdomain.html.in  | 21 +
>  docs/schemas/domaincommon.rng  |  5 +++
>  src/conf/domain_conf.c | 22 +-
>  src/conf/domain_conf.h |  1 +
>  src/qemu/qemu_command.c|  3 +-
>  src/qemu/qemu_hotplug.c|  3 +-
>  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
>  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
> ++
>  .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml|  2 +-
>  tests/qemuxml2argvtest.c   |  3 ++
>  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
>  .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml  |  2 +-
>  tests/qemuxml2xmltest.c|  1 +
>  13 files changed, 139 insertions(+), 5 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..6145d65 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6204,6 +6204,27 @@ 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 TLS is controlled by the
> +  chardev_tls setting in file /etc/libvirt/qemu.conf. If
> +  enabled, then by setting this attribute to "no" will disable usage
> +  of the TLS environment for this particular serial TCP data channel.
> +

Previous discussion:

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

So now there is no functional issue if we agree that this design is what we
want.  I personally thing that there could be also use-case where you want to
configure certificates in qemu.conf and use 'tls' attribute to explicitly
enable TLS encryption only for some VMs and only for some chardev and this is
not currently possible whit this design.  Now user can enable the TLS encryption
for every chardev for every domain and explicitly disable for some chardevs.

This would require to add all the extra code from that discussion to handle
migration properly and that's why I've started the discussion.

> +
> +  ...
> +  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 93b34e0..9062544 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:
> @@ -10038,6 +10040,7 @@ 
> virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>  char *master = NULL;
>  char *slave = NULL;
>  char *append = NULL;
> +char *haveTLS = NULL;
>  int remaining = 0;
>  
>  while (cur != NULL) {
> @@ -10045,6 +10048,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:
> @@ -10221,6 +10226,15 @@ 
> virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>

Re: [libvirt] [PATCH 2/2] Forbid new-line char in name of new networks

2016-10-17 Thread Michal Privoznik 
On 14.10.2016 04:53, Sławek Kapłoński wrote:
> 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 | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index b2af482..df85884 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2973,6 +2973,12 @@ networkValidate(virNetworkDriverStatePtr driver,
>  bool bandwidthAllowed = true;
>  bool usesInterface = false, usesAddress = false;
>  
> +if (virStringHasChars(def->name, "\n")) {
> +virReportError(VIR_ERR_XML_ERROR,
> +   _("name %s cannot contain '\\n'"), def->name);
> +return -1;
> +}
> +
>  /* Only the three L3 network types that are configured by libvirt
>   * need to have a bridge device name / mac address provided
>   */
> 

Good, you found the best place to have this check. Impressive. But if we
go with my suggestion, this can be reduced to:

if (virStringHasChars(def->name, "\n"))
  return -1;


Also, any plans on turning some other checks in other drivers into using
this wrapper?

Michal

--
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

2016-10-17 Thread Michal Privoznik 
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.

> +
> +
>  static const char control_chars[] =
>  "\x01\x02\x03\x04\x05\x06\x07"
>  "\x08" /* \t \n */ "\x0B\x0C" /* \r */ "\x0E\x0F"
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index 8854d5f..7f2c96d 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -272,6 +272,7 @@ char *virStringReplace(const char *haystack,
>  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>  
>  void virStringStripIPv6Brackets(char *str);
> +bool virStringHasChars(const char *str, const char *chars);
>  bool virStringHasControlChars(const char *str);
>  void virStringStripControlChars(char *str);
>  
> 

Michal

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

Re: [libvirt] [PATCH v3 0/2] Forbid new-line char in name of networks

2016-10-17 Thread Michal Privoznik 
On 14.10.2016 04:53, Sławek Kapłoński wrote:
> v2: http://www.redhat.com/archives/libvir-list/2016-October/msg00451.html
> 
> Differences in v3:
> * function to check string moved from src/util/virxml to src/util/virstring
> * validation if name of network contains \n char moved from parsing XML to
>   functions responsible for create/define new networks
> 
> Sławek Kapłoński (2):
>   util: Add function to check if string contains some chars
>   Forbid new-line char in name of new networks
> 
>  src/conf/network_conf.c | 2 +-
>  src/libvirt_private.syms| 1 +
>  src/network/bridge_driver.c | 6 ++
>  src/util/virstring.c| 9 +
>  src/util/virstring.h| 1 +
>  5 files changed, 18 insertions(+), 1 deletion(-)
> 

Looks more like it. I mean, we are nearly there. See my comments to each
patch.

Michal

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

[libvirt] [PULL 20/21] target-i386: Return runnability information on query-cpu-definitions

2016-10-17 Thread Eduardo Habkost 
Fill the "unavailable-features" field on the x86 implementation
of query-cpu-definitions.

Cc: Jiri Denemark 
Cc: libvir-list@redhat.com
Reviewed-by: Igor Mammedov 
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 76 +++
 1 file changed, 76 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 19b8cc4..754e575 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s)
 }
 }
 
+/* Return the feature property name for a feature flag bit */
+static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
+{
+/* XSAVE components are automatically enabled by other features,
+ * so return the original feature name instead
+ */
+if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) {
+int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr;
+
+if (comp < ARRAY_SIZE(x86_ext_save_areas) &&
+x86_ext_save_areas[comp].bits) {
+w = x86_ext_save_areas[comp].feature;
+bitnr = ctz32(x86_ext_save_areas[comp].bits);
+}
+}
+
+assert(bitnr < 32);
+assert(w < FEATURE_WORDS);
+return feature_word_info[w].feat_names[bitnr];
+}
+
 /* Compatibily hack to maintain legacy +-feat semantic,
  * where +-feat overwrites any feature set by
  * feat=on|feat even if the later is parsed after +-feat
@@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char 
*typename, char *features,
 }
 }
 
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
+static int x86_cpu_filter_features(X86CPU *cpu);
+
+/* Check for missing features that may prevent the CPU class from
+ * running using the current machine and accelerator.
+ */
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
+ strList **missing_feats)
+{
+X86CPU *xc;
+FeatureWord w;
+Error *err = NULL;
+strList **next = missing_feats;
+
+if (xcc->kvm_required && !kvm_enabled()) {
+strList *new = g_new0(strList, 1);
+new->value = g_strdup("kvm");;
+*missing_feats = new;
+return;
+}
+
+xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc;
+
+x86_cpu_load_features(xc, );
+if (err) {
+/* Errors at x86_cpu_load_features should never happen,
+ * but in case it does, just report the model as not
+ * runnable at all using the "type" property.
+ */
+strList *new = g_new0(strList, 1);
+new->value = g_strdup("type");
+*next = new;
+next = >next;
+}
+
+x86_cpu_filter_features(xc);
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+uint32_t filtered = xc->filtered_features[w];
+int i;
+for (i = 0; i < 32; i++) {
+if (filtered & (1UL << i)) {
+strList *new = g_new0(strList, 1);
+new->value = g_strdup(x86_cpu_feature_name(w, i));
+*next = new;
+next = >next;
+}
+}
+}
+
+object_unref(OBJECT(xc));
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2122,6 +2196,8 @@ static void x86_cpu_definition_entry(gpointer data, 
gpointer user_data)
 
 info = g_malloc0(sizeof(*info));
 info->name = x86_cpu_class_get_model_name(cc);
+x86_cpu_class_check_missing_features(cc, >unavailable_features);
+info->has_unavailable_features = true;
 
 entry = g_malloc0(sizeof(*entry));
 entry->value = info;
-- 
2.7.4

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

[libvirt] [PULL 09/21] qmp: Add runnability information to query-cpu-definitions

2016-10-17 Thread Eduardo Habkost 
Add a new optional field to query-cpu-definitions schema:
"unavailable-features". It will contain a list of QOM properties
that prevent the CPU model from running in the current host.

Cc: David Hildenbrand 
Cc: Michael Mueller 
Cc: Christian Borntraeger 
Cc: Cornelia Huck 
Cc: Jiri Denemark 
Cc: libvir-list@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Eduardo Habkost 
---
 qapi-schema.json | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index ded1179..5a8ec38 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3101,10 +3101,31 @@
 #  QEMU version, machine type, machine options and accelerator options.
 #  A static model is always migration-safe. (since 2.8)
 #
+# @unavailable-features: #optional List of properties that prevent
+#the CPU model from running in the current
+#host. (since 2.8)
+#
+# @unavailable-features is a list of QOM property names that
+# represent CPU model attributes that prevent the CPU from running.
+# If the QOM property is read-only, that means there's no known
+# way to make the CPU model run in the current host. Implementations
+# that choose not to provide specific information return the
+# property name "type".
+# If the property is read-write, it means that it MAY be possible
+# to run the CPU model in the current host if that property is
+# changed. Management software can use it as hints to suggest or
+# choose an alternative for the user, or just to generate meaningful
+# error messages explaining why the CPU model can't be used.
+# If @unavailable-features is an empty list, the CPU model is
+# runnable using the current host and machine-type.
+# If @unavailable-features is not present, runnability
+# information for the CPU is not available.
+#
 # Since: 1.2.0
 ##
 { 'struct': 'CpuDefinitionInfo',
-  'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool' } }
+  'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool',
+'*unavailable-features': [ 'str' ] } }
 
 ##
 # @query-cpu-definitions:
-- 
2.7.4

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

Re: [libvirt] [PATCH v9 3/5] qemu: Introduce qemuDomainChardevPrivatePtr

2016-10-17 Thread Pavel Hrdina 
On Fri, Oct 14, 2016 at 04:23:06PM -0400, John Ferlan wrote:
> Modeled after the qemuDomainHostdevPrivatePtr (commit id '27726d8c'),
> create a privateData pointer in the _virDomainChardevDef to allow storage
> of private data for a hypervisor in order to at least temporarily store
> secret data for usage during qemuBuildCommandLine.
> 
> NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
> expecting to restore the secret data, there's no need to add code
> code to handle this new structure there.
> 
> Signed-off-by: John Ferlan 
> ---

ACK

Pavel


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

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

2016-10-17 Thread Nikolay Shirokovskiy 
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).

Nikolay

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

[libvirt] [PATCH v2 0/2] New libssh transport

2016-10-17 Thread 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 v1 to v2:
- implemented keyboard interactive
- code polish, and fixes


Pino Toscano (2):
  virNetSocket: allow to not close FD
  libssh_transport: add new libssh-based 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|  184 +-
 src/rpc/virnetsocket.h|   13 +
 src/util/virerror.c   |9 +-
 14 files changed, 1959 insertions(+), 4 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

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

2016-10-17 Thread Pino Toscano 
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

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/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/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/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)  \

[libvirt] [PATCH v2 1/2] virNetSocket: allow to not close FD

2016-10-17 Thread 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

Re: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev

2016-10-17 Thread Pavel Hrdina 
On Mon, Oct 17, 2016 at 09:54:46AM -0400, John Ferlan wrote:
> 
> 
> On 10/17/2016 04:09 AM, Pavel Hrdina wrote:
> > On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
> >> Add an optional "tls='yes|no'" attribute for a TCP chardev for the
> >> express purpose to disable setting up TLS for the specific chardev in
> >> the event the qemu.conf settings have enabled hypervisor wide TLS for
> >> serial TCP chardevs.
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  docs/formatdomain.html.in  | 21 +
> >>  docs/schemas/domaincommon.rng  |  5 +++
> >>  src/conf/domain_conf.c | 22 +-
> >>  src/conf/domain_conf.h |  1 +
> >>  src/qemu/qemu_command.c|  3 +-
> >>  src/qemu/qemu_hotplug.c|  3 +-
> >>  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
> >>  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
> >> ++
> >>  .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml|  2 +-
> >>  tests/qemuxml2argvtest.c   |  3 ++
> >>  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
> >>  .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml  |  2 +-
> >>  tests/qemuxml2xmltest.c|  1 +
> >>  13 files changed, 139 insertions(+), 5 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..6145d65 100644
> >> --- a/docs/formatdomain.html.in
> >> +++ b/docs/formatdomain.html.in
> >> @@ -6204,6 +6204,27 @@ 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 TLS is controlled by the
> >> +  chardev_tls setting in file /etc/libvirt/qemu.conf. If
> >> +  enabled, then by setting this attribute to "no" will disable usage
> >> +  of the TLS environment for this particular serial TCP data channel.
> >> +
> > 
> > Previous discussion:
> > 
> > http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
> > 
> > So now there is no functional issue if we agree that this design is what we
> > want.  I personally thing that there could be also use-case where you want 
> > to
> > configure certificates in qemu.conf and use 'tls' attribute to explicitly
> > enable TLS encryption only for some VMs and only for some chardev and this 
> > is
> > not currently possible whit this design.  Now user can enable the TLS 
> > encryption
> > for every chardev for every domain and explicitly disable for some chardevs.
> > 
> > This would require to add all the extra code from that discussion to handle
> > migration properly and that's why I've started the discussion.
> > 
> 
> w/r/t: design - re-read what I pulled from Dan's v5 review:
> 
> http://www.redhat.com/archives/libvir-list/2016-October/msg00698.html
> 
> While forcing to set "tls='yes'" is certainly a mechanism that could be
> used and adding extra code is possible, is that something that we want
> to require of everyone that's using TLS chardev's? If you go through the
> trouble of configuring for your host, then how would you feel about
> having to update all your domains (for those that have more than 1 or 2)
> to set the property in order to be able to use it? Again, I really don't

I have a feeling that we are having trouble to understand each other.  I'm not
saying that you will have to set "tls='yes'" for every domain and every chardev.
Forcing to set "tls='no'" for every domain and every chardevs if you want to use
TLS only for one domain is not a good mechanism.

What I would like to achieve is this:

1. Currently there is already existing "chardev_tls" config option that allows
you to enable/disable TLS for all domain and all chardevs

2. This patch improves the current functionality by adding an option to
explicitly disable TLS for some chardves by using "tls='no'" if "chardev_tls"
is set to "1".

3. My additional proposal is to add another improvement to current functionality
by allowing to explicitly enable TLS for some chardevs by using "tls='yes'" if
"chardev_tls" is set to "0".  And I can see the use-case, you have a shared host
between several people and only one of them would like to use TLS encryption for
his domain and not affect other users so he

Re: [libvirt] [PATCH v9 5/5] qemu: Add the ability to hotplug a secret object for TCP chardev TLS

2016-10-17 Thread Pavel Hrdina 
On Fri, Oct 14, 2016 at 04:23:08PM -0400, John Ferlan wrote:
> 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 | 62 
> +
>  src/qemu/qemu_hotplug.h |  3 ++-
>  tests/qemuhotplugtest.c |  2 +-
>  4 files changed, 61 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 aad7fa1..69d562f 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,30 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
>  if (qemuDomainChrPreInsert(vmdef, chr) < 0)
>  goto cleanup;
>  
> +if (qemuDomainSecretChardevPrepare(conn, driver, priv, chr) < 0)
> +goto cleanup;
> +
>  if (cfg->chardevTLS &&
>  dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
> +qemuDomainChardevPrivatePtr chardevPriv =
> +QEMU_DOMAIN_CHARDEV_PRIVATE(chr);
> +qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
> +
> +/* Add a secret object in order to access the TLS environment.
> + * The secinfo will only be created for serial TCP device. */
> +if (secinfo) {
> +if (qemuBuildSecretInfoProps(secinfo, ) < 0)
> +goto cleanup;
> +
> +if (!(secAlias = qemuDomainGetSecretAESAlias(chr->info.alias,
> + false)))
> +goto cleanup;
> +}
> +
>  if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
>   dev->data.tcp.listen,
>   cfg->chardevTLSx509verify,
> - NULL,
> + secAlias,
>   priv->qemuCaps,
>   ) < 0)
>  goto cleanup;
> @@ -1745,6 +1767,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 +1806,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 +1815,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 +4422,7 @@ int

Re: [libvirt] [PATCH v9 4/5] qemu: Add a secret object to/for a chardev tcp with secret

2016-10-17 Thread John Ferlan 


On 10/17/2016 10:11 AM, Pavel Hrdina wrote:
> On Fri, Oct 14, 2016 at 04:23:07PM -0400, John Ferlan wrote:
>> 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| 32 ++-
>>  src/qemu/qemu_command.h|  1 +
>>  src/qemu/qemu_domain.c | 99 
>> +-
>>  src/qemu/qemu_domain.h | 16 +++-
>>  src/qemu/qemu_hotplug.c|  1 +
>>  src/qemu/qemu_process.c|  6 +-
>>  ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 +
>>  ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++
>>  tests/qemuxml2argvtest.c   | 18 
>>  9 files changed, 252 insertions(+), 9 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 aaf7018..b2dfee0 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;
>>  }
>>  
>> @@ -4946,6 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>>  if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
>>  dev->data.tcp.listen,
>>  cfg->chardevTLSx509verify,
>> +!!cfg->chardevTLSx509secretUUID,
>>  alias, qemuCaps) < 0)
>>  goto error;
>>  
>> @@ -8542,6 +8557,19 @@ qemuBuildSerialCommandLine(virLogManagerPtr 
>> logManager,
>>  
>>  /* Use -chardev with -device if they are available */
>>  if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) {
>> +qemuDomainChardevPrivatePtr chardevPriv =
>> +

Re: [libvirt] [PATCH v9 4/5] qemu: Add a secret object to/for a chardev tcp with secret

2016-10-17 Thread Pavel Hrdina 
On Fri, Oct 14, 2016 at 04:23:07PM -0400, John Ferlan wrote:
> 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| 32 ++-
>  src/qemu/qemu_command.h|  1 +
>  src/qemu/qemu_domain.c | 99 
> +-
>  src/qemu/qemu_domain.h | 16 +++-
>  src/qemu/qemu_hotplug.c|  1 +
>  src/qemu/qemu_process.c|  6 +-
>  ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 +
>  ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++
>  tests/qemuxml2argvtest.c   | 18 
>  9 files changed, 252 insertions(+), 9 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 aaf7018..b2dfee0 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;
>  }
>  
> @@ -4946,6 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
>  dev->data.tcp.listen,
>  cfg->chardevTLSx509verify,
> +!!cfg->chardevTLSx509secretUUID,
>  alias, qemuCaps) < 0)
>  goto error;
>  
> @@ -8542,6 +8557,19 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
>  
>  /* Use -chardev with -device if they are available */
>  if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) {
> +qemuDomainChardevPrivatePtr chardevPriv =
> +QEMU_DOMAIN_CHARDEV_PRIVATE(serial);
> +qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
> +
> +/* Add the secret object first if

Re: [libvirt] [PATCH v9 2/5] conf: Introduce {default|chardev}_tls_x509_secret_uuid

2016-10-17 Thread John Ferlan 


On 10/17/2016 06:52 AM, Pavel Hrdina wrote:
> On Fri, Oct 14, 2016 at 04:23:05PM -0400, John Ferlan wrote:
>> Add a new qemu.conf variables to store the UUID for the secret that could
>> be used to present credentials to access the TLS chardev.  Since this will
>> be a server level and it's possible to use some sort of default, introduce
>> both the default and chardev logic at the same time making the setting of
>> the chardev check for it's own value, then if not present checking whether
>> the default value had been set.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/qemu/libvirtd_qemu.aug |  2 ++
>>  src/qemu/qemu.conf | 24 
>>  src/qemu/qemu_conf.c   | 14 ++
>>  src/qemu/qemu_conf.h   |  2 ++
>>  src/qemu/test_libvirtd_qemu.aug.in |  2 ++
>>  5 files changed, 44 insertions(+)
>>
>> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
>> index 988201e..73ebeda 100644
>> --- a/src/qemu/libvirtd_qemu.aug
>> +++ b/src/qemu/libvirtd_qemu.aug
>> @@ -29,6 +29,7 @@ module Libvirtd_qemu =
>> (* Config entry grouped by function - same order as example config *)
>> let default_tls_entry = str_entry "default_tls_x509_cert_dir"
>>   | bool_entry "default_tls_x509_verify"
>> + | str_entry "default_tls_x509_secret_uuid"
>>  
>> let vnc_entry = str_entry "vnc_listen"
>>   | bool_entry "vnc_auto_unix_socket"
>> @@ -51,6 +52,7 @@ module Libvirtd_qemu =
>> let chardev_entry = bool_entry "chardev_tls"
>>   | str_entry "chardev_tls_x509_cert_dir"
>>   | bool_entry "chardev_tls_x509_verify"
>> + | str_entry "chardev_tls_x509_secret_uuid"
>>  
>> let nogfx_entry = bool_entry "nographics_allow_host_audio"
>>  
>> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
>> index e4c2aae..493c171 100644
>> --- a/src/qemu/qemu.conf
>> +++ b/src/qemu/qemu.conf
>> @@ -28,6 +28,20 @@
>>  #
>>  #default_tls_x509_verify = 1
>>  
>> +#
>> +# Libvirt assumes the server-key.pem file is unencrypted by default.
>> +# To use an encrypted server-key.pem file, the password to decrypt the
> 
> You've forgot to remove the extra "the".
> 

Weird - I konw I made the change... where'd it go...


>> +# the PEM file is required. This can be provided by creating a secret
>> +# object in libvirt and then to uncomment this setting to set the UUID
>> +# of the secret.
>> +#
>> +# NB This default all-zeros UUID will not work. Replace it with the
>> +# output from the UUID for the TLS secret from a 'virsh secret-list'
>> +# command and then uncomment the entry
>> +#
>> +#default_tls_x509_secret_uuid = "----"
>> +
>> +
>>  # VNC is configured to listen on 127.0.0.1 by default.
>>  # To make it listen on all public interfaces, uncomment
>>  # this next option.
>> @@ -214,6 +228,16 @@
>>  #chardev_tls_x509_verify = 1
>>  
>>  
>> +# Uncomment and use the following option to override the default secret
>> +# uuid provided in the default_tls_x509_secret_uuid parameter.
> 
> s/uuid/UUID/
> 
> ACK
> 

change - thanks

John

[...]

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

Re: [libvirt] [PATCH v9 2/5] conf: Introduce {default|chardev}_tls_x509_secret_uuid

2016-10-17 Thread Pavel Hrdina 
On Fri, Oct 14, 2016 at 04:23:05PM -0400, John Ferlan wrote:
> Add a new qemu.conf variables to store the UUID for the secret that could
> be used to present credentials to access the TLS chardev.  Since this will
> be a server level and it's possible to use some sort of default, introduce
> both the default and chardev logic at the same time making the setting of
> the chardev check for it's own value, then if not present checking whether
> the default value had been set.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/libvirtd_qemu.aug |  2 ++
>  src/qemu/qemu.conf | 24 
>  src/qemu/qemu_conf.c   | 14 ++
>  src/qemu/qemu_conf.h   |  2 ++
>  src/qemu/test_libvirtd_qemu.aug.in |  2 ++
>  5 files changed, 44 insertions(+)
> 
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 988201e..73ebeda 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -29,6 +29,7 @@ module Libvirtd_qemu =
> (* Config entry grouped by function - same order as example config *)
> let default_tls_entry = str_entry "default_tls_x509_cert_dir"
>   | bool_entry "default_tls_x509_verify"
> + | str_entry "default_tls_x509_secret_uuid"
>  
> let vnc_entry = str_entry "vnc_listen"
>   | bool_entry "vnc_auto_unix_socket"
> @@ -51,6 +52,7 @@ module Libvirtd_qemu =
> let chardev_entry = bool_entry "chardev_tls"
>   | str_entry "chardev_tls_x509_cert_dir"
>   | bool_entry "chardev_tls_x509_verify"
> + | str_entry "chardev_tls_x509_secret_uuid"
>  
> let nogfx_entry = bool_entry "nographics_allow_host_audio"
>  
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index e4c2aae..493c171 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -28,6 +28,20 @@
>  #
>  #default_tls_x509_verify = 1
>  
> +#
> +# Libvirt assumes the server-key.pem file is unencrypted by default.
> +# To use an encrypted server-key.pem file, the password to decrypt the

You've forgot to remove the extra "the".

> +# the PEM file is required. This can be provided by creating a secret
> +# object in libvirt and then to uncomment this setting to set the UUID
> +# of the secret.
> +#
> +# NB This default all-zeros UUID will not work. Replace it with the
> +# output from the UUID for the TLS secret from a 'virsh secret-list'
> +# command and then uncomment the entry
> +#
> +#default_tls_x509_secret_uuid = "----"
> +
> +
>  # VNC is configured to listen on 127.0.0.1 by default.
>  # To make it listen on all public interfaces, uncomment
>  # this next option.
> @@ -214,6 +228,16 @@
>  #chardev_tls_x509_verify = 1
>  
>  
> +# Uncomment and use the following option to override the default secret
> +# uuid provided in the default_tls_x509_secret_uuid parameter.

s/uuid/UUID/

ACK

> +#
> +# NB This default all-zeros UUID will not work. Replace it with the
> +# output from the UUID for the TLS secret from a 'virsh secret-list'
> +# command and then uncomment the entry
> +#
> +#chardev_tls_x509_secret_uuid = "----"
> +
> +
>  # By default, if no graphical front end is configured, libvirt will disable
>  # QEMU audio output since directly talking to alsa/pulseaudio may not work
>  # with various security settings. If you know what you're doing, enable
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 635fa27..109668b 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -365,6 +365,7 @@ static void virQEMUDriverConfigDispose(void *obj)
>  VIR_FREE(cfg->nvramDir);
>  
>  VIR_FREE(cfg->defaultTLSx509certdir);
> +VIR_FREE(cfg->defaultTLSx509secretUUID);
>  
>  VIR_FREE(cfg->vncTLSx509certdir);
>  VIR_FREE(cfg->vncListen);
> @@ -377,6 +378,7 @@ static void virQEMUDriverConfigDispose(void *obj)
>  VIR_FREE(cfg->spiceSASLdir);
>  
>  VIR_FREE(cfg->chardevTLSx509certdir);
> +VIR_FREE(cfg->chardevTLSx509secretUUID);
>  
>  while (cfg->nhugetlbfs) {
>  cfg->nhugetlbfs--;
> @@ -446,6 +448,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr 
> cfg,
>  goto cleanup;
>  if (virConfGetValueBool(conf, "default_tls_x509_verify", 
> >defaultTLSx509verify) < 0)
>  goto cleanup;
> +if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
> +  >defaultTLSx509secretUUID) < 0)
> +goto cleanup;
> +
>  if (virConfGetValueBool(conf, "vnc_auto_unix_socket", 
> >vncAutoUnixSocket) < 0)
>  goto cleanup;
>  if (virConfGetValueBool(conf, "vnc_tls", >vncTLS) < 0)
> @@ -513,6 +519,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr 
> cfg,
>  goto cleanup;
>  if (rv == 0)
>  cfg->chardevTLSx509verify = cfg->defaultTLSx509verify;
> +if (virConfGetValueString(conf,

Re: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev

2016-10-17 Thread John Ferlan 


On 10/17/2016 04:09 AM, Pavel Hrdina wrote:
> On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
>> Add an optional "tls='yes|no'" attribute for a TCP chardev for the
>> express purpose to disable setting up TLS for the specific chardev in
>> the event the qemu.conf settings have enabled hypervisor wide TLS for
>> serial TCP chardevs.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  docs/formatdomain.html.in  | 21 +
>>  docs/schemas/domaincommon.rng  |  5 +++
>>  src/conf/domain_conf.c | 22 +-
>>  src/conf/domain_conf.h |  1 +
>>  src/qemu/qemu_command.c|  3 +-
>>  src/qemu/qemu_hotplug.c|  3 +-
>>  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
>>  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
>> ++
>>  .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml|  2 +-
>>  tests/qemuxml2argvtest.c   |  3 ++
>>  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
>>  .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml  |  2 +-
>>  tests/qemuxml2xmltest.c|  1 +
>>  13 files changed, 139 insertions(+), 5 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..6145d65 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -6204,6 +6204,27 @@ 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 TLS is controlled by the
>> +  chardev_tls setting in file /etc/libvirt/qemu.conf. If
>> +  enabled, then by setting this attribute to "no" will disable usage
>> +  of the TLS environment for this particular serial TCP data channel.
>> +
> 
> Previous discussion:
> 
> http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
> 
> So now there is no functional issue if we agree that this design is what we
> want.  I personally thing that there could be also use-case where you want to
> configure certificates in qemu.conf and use 'tls' attribute to explicitly
> enable TLS encryption only for some VMs and only for some chardev and this is
> not currently possible whit this design.  Now user can enable the TLS 
> encryption
> for every chardev for every domain and explicitly disable for some chardevs.
> 
> This would require to add all the extra code from that discussion to handle
> migration properly and that's why I've started the discussion.
> 

w/r/t: design - re-read what I pulled from Dan's v5 review:

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

While forcing to set "tls='yes'" is certainly a mechanism that could be
used and adding extra code is possible, is that something that we want
to require of everyone that's using TLS chardev's? If you go through the
trouble of configuring for your host, then how would you feel about
having to update all your domains (for those that have more than 1 or 2)
to set the property in order to be able to use it? Again, I really don't
think it's a good idea to be mucking around with changing XML of running
domains, adding extra checks in the migration code, altering the
format/parse code to check flags, and/or anything else that may come up.
The less we do that the better - introduces less risk for making or
missing assumptions.

For comparison spice uses an attribute mechanism "tlsPort={-1|0|port}"
in order to use TLS, but it also checks cfg->spiceTLS. Alternatively,
vnc will just take the cfg->vncTLS to decide whether to use or not
(e.g., there's no way to avoid).

So one of the existing mechanisms forces you to define something to use
TLS configured for the host and the other doesn't. Spice has the
additional configuration option to determine specific channels by name
that would use the secure port.

This patch can still be considered "outside" of the subsequent 4 in this
series...

>> +
>> +  ...
>> +  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

Re: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev

2016-10-17 Thread John Ferlan 


On 10/17/2016 10:37 AM, Pavel Hrdina wrote:
> On Mon, Oct 17, 2016 at 09:54:46AM -0400, John Ferlan wrote:
>>
>>
>> On 10/17/2016 04:09 AM, Pavel Hrdina wrote:
>>> On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
 Add an optional "tls='yes|no'" attribute for a TCP chardev for the
 express purpose to disable setting up TLS for the specific chardev in
 the event the qemu.conf settings have enabled hypervisor wide TLS for
 serial TCP chardevs.

 Signed-off-by: John Ferlan 
 ---
  docs/formatdomain.html.in  | 21 +
  docs/schemas/domaincommon.rng  |  5 +++
  src/conf/domain_conf.c | 22 +-
  src/conf/domain_conf.h |  1 +
  src/qemu/qemu_command.c|  3 +-
  src/qemu/qemu_hotplug.c|  3 +-
  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
 ++
  .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml|  2 +-
  tests/qemuxml2argvtest.c   |  3 ++
  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
  .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml  |  2 +-
  tests/qemuxml2xmltest.c|  1 +
  13 files changed, 139 insertions(+), 5 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..6145d65 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -6204,6 +6204,27 @@ 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 TLS is controlled by the
 +  chardev_tls setting in file /etc/libvirt/qemu.conf. If
 +  enabled, then by setting this attribute to "no" will disable usage
 +  of the TLS environment for this particular serial TCP data channel.
 +
>>>
>>> Previous discussion:
>>>
>>> http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
>>>
>>> So now there is no functional issue if we agree that this design is what we
>>> want.  I personally thing that there could be also use-case where you want 
>>> to
>>> configure certificates in qemu.conf and use 'tls' attribute to explicitly
>>> enable TLS encryption only for some VMs and only for some chardev and this 
>>> is
>>> not currently possible whit this design.  Now user can enable the TLS 
>>> encryption
>>> for every chardev for every domain and explicitly disable for some chardevs.
>>>
>>> This would require to add all the extra code from that discussion to handle
>>> migration properly and that's why I've started the discussion.
>>>
>>
>> w/r/t: design - re-read what I pulled from Dan's v5 review:
>>
>> http://www.redhat.com/archives/libvir-list/2016-October/msg00698.html
>>
>> While forcing to set "tls='yes'" is certainly a mechanism that could be
>> used and adding extra code is possible, is that something that we want
>> to require of everyone that's using TLS chardev's? If you go through the
>> trouble of configuring for your host, then how would you feel about
>> having to update all your domains (for those that have more than 1 or 2)
>> to set the property in order to be able to use it? Again, I really don't
> 
> I have a feeling that we are having trouble to understand each other.  I'm not
> saying that you will have to set "tls='yes'" for every domain and every 
> chardev.
> Forcing to set "tls='no'" for every domain and every chardevs if you want to 
> use
> TLS only for one domain is not a good mechanism.

Clearly ;-)  Setting "tls='yes'" wasn't my implication either. The way
this patch is written 'YES' or 'ABSENT' have the same meaning to take
the host default.

So "forcing" setting of "tls='no'"
> 
> What I would like to achieve is this:
> 
> 1. Currently there is already existing "chardev_tls" config option that allows
> you to enable/disable TLS for all domain and all chardevs
> 
> 2. This patch improves the current functionality by adding an option to
> explicitly disable TLS for some chardves by using "tls='no'" if "chardev_tls"
> is set to "1".
> 
> 3. My additional proposal is to add another improvement to current 
>

[libvirt] [PATCH 3/3] qemu: Remove unnecessary NULL arg check

2016-10-17 Thread John Ferlan 
qemuDomainSecret{Disk|Hostdev}Prepare has a prototype that checks for
ATTRIBUTE_NONNULL(1) for 'conn'.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_domain.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0c9416f..5e9f08f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1062,9 +1062,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
 qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
 qemuDomainSecretInfoPtr secinfo = NULL;
 
-if (!conn)
-return 0;
-
 if (qemuDomainSecretDiskCapable(src)) {
 virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI;
 
@@ -1147,7 +1144,7 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
 virDomainHostdevSubsysPtr subsys = >source.subsys;
 qemuDomainSecretInfoPtr secinfo = NULL;
 
-if (conn && hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
 subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
 
 virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
-- 
2.7.4

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

[libvirt] [PATCH 2/3] qemu: Add 'verify-peer=yes' test for chardev TCP TLS

2016-10-17 Thread John Ferlan 
Missing the option to set verify-peer to yes

Signed-off-by: John Ferlan 
---
 ...xml2argv-serial-tcp-tlsx509-chardev-verify.args | 33 +
 ...uxml2argv-serial-tcp-tlsx509-chardev-verify.xml | 41 ++
 tests/qemuxml2argvtest.c   |  5 +++
 3 files changed, 79 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml

diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args 
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
new file mode 100644
index 000..f521e33
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
@@ -0,0 +1,33 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-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 \
+-chardev udp,id=charserial0,host=127.0.0.1,port=,localaddr=127.0.0.1,\
+localport= \
+-device isa-serial,chardev=charserial0,id=serial0 \
+-object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
+endpoint=client,verify-peer=yes \
+-chardev socket,id=charserial1,host=127.0.0.1,port=,\
+tls-creds=objserial1_tls0 \
+-device isa-serial,chardev=charserial1,id=serial1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml
new file mode 100644
index 000..1618b02
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml
@@ -0,0 +1,41 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b1cc4d8..3e9f825 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1162,6 +1162,11 @@ mymain(void)
 DO_TEST("serial-tcp-tlsx509-chardev",
 QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
 QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+driver.config->chardevTLSx509verify = 1;
+DO_TEST("serial-tcp-tlsx509-chardev-verify",
+QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
+QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+driver.config->chardevTLSx509verify = 0;
 driver.config->chardevTLS = 0;
 VIR_FREE(driver.config->chardevTLSx509certdir);
 DO_TEST("serial-many-chardev",
-- 
2.7.4

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

[libvirt] [PATCH 0/3] Misc adjustments from recent code review

2016-10-17 Thread John Ferlan 
The following were all part of the review of the TCP chardev TLS series which
were outside the realm of the specific changes for the series...

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

1. Removal of cfg from qemuProcessPrepareDomain should be separate patch
2. Setting chardevTLSx509verify should have been it's own patch...
3. !conn in qemuDomainSecretChardevPrepare not necessary - so that's
   true for the SecretDiskPrepare and SecretHostdevPrepare too

John Ferlan (3):
  qemu: Remove unnecessary cfg fetch/unref
  qemu: Add 'verify-peer=yes' test for chardev TCP TLS
  qemu: Remove unnecessary NULL arg check

 src/qemu/qemu_domain.c |  5 +--
 src/qemu/qemu_process.c|  2 --
 ...xml2argv-serial-tcp-tlsx509-chardev-verify.args | 33 +
 ...uxml2argv-serial-tcp-tlsx509-chardev-verify.xml | 41 ++
 tests/qemuxml2argvtest.c   |  5 +++
 5 files changed, 80 insertions(+), 6 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml

-- 
2.7.4

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

[libvirt] [PATCH 1/3] qemu: Remove unnecessary cfg fetch/unref

2016-10-17 Thread John Ferlan 
qemuProcessPrepareDomain has no need to fetch/unref the cfg, so remove it.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_process.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0f5a11b..d641f33 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5092,7 +5092,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
 size_t i;
 char *nodeset = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 virCapsPtr caps;
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
@@ -5188,7 +5187,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
  cleanup:
 VIR_FREE(nodeset);
 virObjectUnref(caps);
-virObjectUnref(cfg);
 return ret;
 }
 
-- 
2.7.4

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