[libvirt] [PATCH] qemu:address:Fix segfault in qemuDomainPrimeVirtioDeviceAddresses

2018-11-08 Thread Wang Yechao
On aarch64, lauch vm with the follow configuration:


  
  

  


libvirtd will crash when access the net->model.

Signed-off-by: Wang Yechao 
---
 src/qemu/qemu_domain_address.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 24dd7c1..27c9bfb 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -324,7 +324,8 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def,
 for (i = 0; i < def->nnets; i++) {
 virDomainNetDefPtr net = def->nets[i];
 
-if (STREQ(net->model, "virtio") &&
+if (net->model &&
+STREQ(net->model, "virtio") &&
 net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
 net->info.type = type;
 }
-- 
1.8.3.1

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


Re: [libvirt] [PATCH v3 04/13] security_manager: Rework metadata locking

2018-11-08 Thread John Ferlan



On 10/17/18 5:06 AM, Michal Privoznik wrote:
> Trying to use virlockd to lock metadata turns out to be too big
> gun. Since we will always spawn a separate process for relabeling
> we are safe to use thread unsafe POSIX locks and take out
> virtlockd completely out of the picture.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/security/security_dac.c |  10 +-
>  src/security/security_manager.c | 225 +---
>  src/security/security_manager.h |  17 ++-
>  src/security/security_selinux.c |   9 +-
>  4 files changed, 139 insertions(+), 122 deletions(-)
> 
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 580d800bb1..ad2561a241 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -204,6 +204,7 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
>   void *opaque)
>  {
>  virSecurityDACChownListPtr list = opaque;
> +virSecurityManagerMetadataLockStatePtr state;
>  const char **paths = NULL;
>  size_t npaths = 0;
>  size_t i;
> @@ -216,14 +217,10 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
>  for (i = 0; i < list->nItems; i++) {
>  const char *p = list->items[i]->path;
>  
> -if (!p ||
> -virFileIsDir(p))
> -continue;
> -
>  VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p);
>  }
>  
> -if (virSecurityManagerMetadataLock(list->manager, paths, npaths) < 0)
> +if (!(state = virSecurityManagerMetadataLock(list->manager, paths, 
> npaths)))
>  goto cleanup;
>  
>  for (i = 0; i < list->nItems; i++) {
> @@ -246,8 +243,7 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
>  break;
>  }
>  
> -if (virSecurityManagerMetadataUnlock(list->manager, paths, npaths) < 0)
> -goto cleanup;
> +virSecurityManagerMetadataUnlock(list->manager, );
>  
>  if (rv < 0)
>  goto cleanup;
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index c6c80e6165..b675f8ab46 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -21,6 +21,10 @@
>   */
>  #include 
>  
> +#include 
> +#include 
> +#include 
> +
>  #include "security_driver.h"
>  #include "security_stack.h"
>  #include "security_dac.h"
> @@ -30,14 +34,11 @@
>  #include "virlog.h"
>  #include "locking/lock_manager.h"
>  #include "virfile.h"
> -#include "virtime.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_SECURITY
>  
>  VIR_LOG_INIT("security.security_manager");
>  
> -virMutex lockManagerMutex = VIR_MUTEX_INITIALIZER;
> -
>  struct _virSecurityManager {
>  virObjectLockable parent;
>  
> @@ -47,10 +48,6 @@ struct _virSecurityManager {
>  void *privateData;
>  
>  virLockManagerPluginPtr lockPlugin;
> -/* This is a FD that represents a connection to virtlockd so
> - * that connection is kept open in between MetdataLock() and
> - * MetadataUnlock() calls. */
> -int clientfd;
>  };
>  
>  static virClassPtr virSecurityManagerClass;
> @@ -66,7 +63,6 @@ void virSecurityManagerDispose(void *obj)
>  mgr->drv->close(mgr);
>  
>  virObjectUnref(mgr->lockPlugin);
> -VIR_FORCE_CLOSE(mgr->clientfd);
>  
>  VIR_FREE(mgr->privateData);
>  }
> @@ -119,7 +115,6 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv,
>  mgr->flags = flags;
>  mgr->virtDriver = virtDriver;
>  VIR_STEAL_PTR(mgr->privateData, privateData);
> -mgr->clientfd = -1;
>  
>  if (drv->open(mgr) < 0)
>  goto error;
> @@ -1276,129 +1271,153 @@ 
> virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
>  }
>  
>  
> -static virLockManagerPtr
> -virSecurityManagerNewLockManager(virSecurityManagerPtr mgr,
> - const char * const *paths,
> - size_t npaths)
> +struct _virSecurityManagerMetadataLockState {

LockPrivate?

When I first saw State I had a different thought.

I like this better than the previous model... Keeping track of fds is
like other models used. How do these locks handle restarts? Are the
locks free'd if the daemon dies?

> +size_t nfds;
> +int *fds;
> +};
> +
> +
> +static int
> +cmpstringp(const void *p1, const void *p2)
>  {
> -virLockManagerPtr lock;
> -virLockManagerParam params[] = {
> -{ .type = VIR_LOCK_MANAGER_PARAM_TYPE_UUID,
> -.key = "uuid",
> -},
> -{ .type = VIR_LOCK_MANAGER_PARAM_TYPE_STRING,
> -.key = "name",
> -.value = { .cstr = "libvirtd-sec" },
> -},
> -{ .type = VIR_LOCK_MANAGER_PARAM_TYPE_UINT,
> -.key = "pid",
> -.value = { .iv = getpid() },
> -},
> -};
> -const unsigned int flags = 0;
> -size_t i;
> +const char *s1 = *(char * const *) p1;
> +const char *s2 = *(char * const *) p2;
>  
> -if (virGetHostUUID(params[0].value.uuid) < 0)
> -

Re: [libvirt] [PATCH v3 02/13] virprocess: Make virProcessRunInMountNamespace use virProcessRunInFork

2018-11-08 Thread John Ferlan



On 10/17/18 5:06 AM, Michal Privoznik wrote:
> Both virProcessRunInMountNamespace() and virProcessRunInFork()
> look very similar. De-duplicate the code and make
> virProcessRunInMountNamespace() call virProcessRunInFork().>
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virprocess.c | 62 +--
>  1 file changed, 12 insertions(+), 50 deletions(-)
> 
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 51b9ccb1bb..3304879212 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1073,11 +1073,17 @@ int virProcessGetStartTime(pid_t pid,
>  #endif
>  
>  
> -static int virProcessNamespaceHelper(int errfd,
> - pid_t pid,
> - virProcessNamespaceCallback cb,
> +typedef struct _virProcessNamespaceHelperData virProcessNamespaceHelperData;
> +struct _virProcessNamespaceHelperData {
> +pid_t pid;
> +virProcessNamespaceCallback cb;
> +void *opaque;
> +};
> +
> +static int virProcessNamespaceHelper(pid_t pid ATTRIBUTE_UNUSED,
>   void *opaque)
>  {
> +virProcessNamespaceHelperData *data = opaque;
>  int fd = -1;
>  int ret = -1;
>  VIR_AUTOFREE(char *) path = NULL;

Right after this there's a :

 if (virAsprintf(, "/proc/%lld/ns/mnt", (long long) pid) < 0)
 goto cleanup;

The last arg should be data->pid I believe...

Beyond that - nothing jumps out at me...

Reviewed-by: John Ferlan 

John

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


Re: [libvirt] [PATCH v3 03/13] security: Always spawn process for transactions

2018-11-08 Thread John Ferlan



On 10/17/18 5:06 AM, Michal Privoznik wrote:
> In the next commit the virSecurityManagerMetadataLock() is going
> to be turned thread unsafe. Therefore, we have to spawn a
> separate process for it. Always.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/security/security_dac.c | 2 +-
>  src/security/security_selinux.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Based slightly upon the KVM Forum presentation detailing some
performance issues related to the usage of virFork for
virQEMUCapsIsValid and calls to virFileAccessibleAs:

https://www.redhat.com/archives/libvir-list/2018-October/msg01333.html

Given that this patch would thus virFork "always", what kind of impact
could that have? Have you been able to do any performance profiling?
What would cause a single round of SELinux & DAC settings?

I know in an earlier patch I wasn't including performance of virFork
because I believed that the only time it would be used would be for
metadata locking when (re)labeling would be batched and for that case
the "one off" virFork would seem reasonable.

Since it is possible to turn off the NS code and thus go through the
direct call, I think there's "room for it" here too for those singular
cases we could use "pid == -1" to indicate direct calls without virFork
and "pid == 0" to batch together calls using virProcessRunInFork.

That way when you *know* you are batching together more than singular
changes, then the caller can choose to run in a fork knowing the
possible performance penalty, but with the benefits. For those that are
batched we're stuck, but my memory on all the metadata locking paths is
fuzzy already.

What's here does work, but after that KVM Forum preso I guess we
definitely need to pay attention to areas that can be perf bottlenecks
for paths that may be really common.

Having a way to disable or avoid virFork is something we may just need
to have. I'm willing to be convinced otherwise - I'm just "wary" now.


> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index da4a6c72fe..580d800bb1 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c

The function description would need an update way to describe this @pid
usage which differs from the current description.

> @@ -563,7 +563,7 @@ virSecurityDACTransactionCommit(virSecurityManagerPtr mgr 
> ATTRIBUTE_UNUSED,
>  }
>  
>  if ((pid == -1 &&
> - virSecurityDACTransactionRun(pid, list) < 0) ||
> + virProcessRunInFork(virSecurityDACTransactionRun, list) < 0) ||
>  (pid != -1 &&
>   virProcessRunInMountNamespace(pid,
> virSecurityDACTransactionRun,

I think I've previously disclosed my dislike of the format, why not:

if (pid > 0) {
rc = virProcessRunInMountNamespace(pid, ...);
} else {
if (pid == -1)
rc = virSecurityDACTransactionRun(-pid, list);
else
rc = virProcessRunInFork(virSecurityDACTransactionRun, list));
}
if (rc < 0)
goto cleanup;

to me that's a lot cleaner and doesn't involve trying to look at
multiple if statements with ||'s.

> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 467d1e6bfe..0e24b9b3ca 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1104,7 +1104,7 @@ 
> virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr 
> ATTRIBUTE_UNUSED,
>  }
>  
>  if ((pid == -1 &&
> - virSecuritySELinuxTransactionRun(pid, list) < 0) ||
> + virProcessRunInFork(virSecuritySELinuxTransactionRun, list) < 0) ||
>  (pid != -1 &&
>   virProcessRunInMountNamespace(pid,
> virSecuritySELinuxTransactionRun,
> 

ditto.

John

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


Re: [libvirt] [PATCH v3 01/13] virprocess: Introduce virProcessRunInFork

2018-11-08 Thread John Ferlan



On 10/17/18 5:06 AM, Michal Privoznik wrote:
> This new helper can be used to spawn a child process and run
> passed callback from it. This will come handy esp. if the
> callback is not thread safe.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virprocess.c| 81 
>  src/util/virprocess.h| 16 
>  3 files changed, 98 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 335210c31d..e2dc85310a 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2625,6 +2625,7 @@ virProcessKill;
>  virProcessKillPainfully;
>  virProcessKillPainfullyDelay;
>  virProcessNamespaceAvailable;
> +virProcessRunInFork;
>  virProcessRunInMountNamespace;
>  virProcessSchedPolicyTypeFromString;
>  virProcessSchedPolicyTypeToString;
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 3883c708fc..51b9ccb1bb 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1165,6 +1165,87 @@ virProcessRunInMountNamespace(pid_t pid,
>  }
>  
>  
> +static int
> +virProcessForkHelper(int errfd,

For consistency, virProcessRunInForkHelper

> + pid_t pid,

s/pid/parent  (or ppid, IDC)

just to be clear

> + virProcessForkCallback cb,
> + void *opaque)
> +{
> +if (cb(pid, opaque) < 0) {
> +virErrorPtr err = virGetLastError();
> +if (err) {
> +size_t len = strlen(err->message) + 1;
> +ignore_value(safewrite(errfd, err->message, len));
> +}
> +
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +
> +/**
> + * virProcessRunInFork:
> + * @cb: callback to run
> + * @opaque: opaque data to @cb
> + *
> + * Do the fork and run @cb in the child. This can be used when
> + * @cb does something thread unsafe, for instance. However, due
> + * to possible signal propagation @cb should only call async
> + * signal safe functions. On return, the returned value is either
> + * -1 with error message reported if something went bad in the
> + *  parent, if child has died from a signal or if the child
> + *  returned EXIT_CANCELED. Otherwise the returned value is the
> + *  exit status of the child.
> + */
> +int
> +virProcessRunInFork(virProcessForkCallback cb,
> +void *opaque)
> +{
> +int ret = -1;
> +pid_t child = -1;
> +pid_t parent = getpid();
> +int errfd[2] = { -1, -1 };
> +
> +if (pipe2(errfd, O_CLOEXEC) < 0) {
> +virReportSystemError(errno, "%s",
> + _("Cannot create pipe for child"));
> +return -1;
> +}
> +
> +if ((child = virFork()) < 0)
> +goto cleanup;
> +
> +if (child == 0) {
> +VIR_FORCE_CLOSE(errfd[0]);
> +ret = virProcessForkHelper(errfd[1], parent, cb, opaque);
> +VIR_FORCE_CLOSE(errfd[1]);
> +_exit(ret < 0 ? EXIT_CANCELED : ret);
> +} else {
> +int status;
> +VIR_AUTOFREE(char *) buf = NULL;
> +
> +VIR_FORCE_CLOSE(errfd[1]);
> +ignore_value(virFileReadHeaderFD(errfd[0], 1024, ));
> +ret = virProcessWait(child, , false);
> +if (!ret) {



> +ret = status == EXIT_CANCELED ? -1 : status;
> +if (ret) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("child reported: %s"),
> +   NULLSTR(buf));
> +}

I agree w/ Daniel regarding @status reporting like virCommandWait, it
looks ugly but gets all the information out at least.

> +}
> +}
> +
> + cleanup:
> +VIR_FORCE_CLOSE(errfd[0]);
> +VIR_FORCE_CLOSE(errfd[1]);
> +return ret;
> +}
> +
> +
>  #if defined(HAVE_SYS_MOUNT_H) && defined(HAVE_UNSHARE)
>  int
>  virProcessSetupPrivateMountNS(void)
> diff --git a/src/util/virprocess.h b/src/util/virprocess.h
> index 5faa0892fe..1bae8c9212 100644
> --- a/src/util/virprocess.h
> +++ b/src/util/virprocess.h
> @@ -93,6 +93,22 @@ int virProcessRunInMountNamespace(pid_t pid,
>virProcessNamespaceCallback cb,
>void *opaque);
>  
> +/**
> + * virProcessForkCallback:
> + * @pid: parent's pid

It's the "fork's" parent pid. Perhaps would be nice to have ppid or
parent - just because pid is used so many times. It's not that important
though.

> + * @opaque: opaque data
> + *
> + * Callback to run in fork()-ed process.
> + *
> + * Returns: 0 on success,
> + * -1 on error (treated as EXIT_CANCELED)
> + */
> +typedef int (*virProcessForkCallback)(pid_t pid,
> +  void *opaque);
> +
> +int virProcessRunInFork(virProcessForkCallback cb,
> +void *opaque);
> +

Since this is defined before virProcessRunInMountNamespace in source,
probably could do the same in this file. It's a type a orderly thing.

and yes, essentially 

Re: [libvirt] [PATCH] news: Add entry for soft reset support in Xen

2018-11-08 Thread Jim Fehlig

On 11/8/18 10:27 AM, Andrea Bolognani wrote:

On Thu, 2018-11-08 at 08:52 -0700, Jim Fehlig wrote:

Signed-off-by: Jim Fehlig 
---

I'm torn on whether this is a new feature, improvement, or even
simply a bug fix...


The categories are quite fuzzy, but that also means that getting it
100% right is not that important :)

Personally I'd lean towards bug fix, since it's something that users
might reasonably have expected to work even before now; 


Thanks!. I moved it to the bug fixes section, tweaked the summary just a bit to 
read more like a bug fix, and pushed.


Regards,
Jim

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


Re: [libvirt] [PATCH] news: Add entry for soft reset support in Xen

2018-11-08 Thread Andrea Bolognani
On Thu, 2018-11-08 at 08:52 -0700, Jim Fehlig wrote:
> Signed-off-by: Jim Fehlig 
> ---
> 
> I'm torn on whether this is a new feature, improvement, or even
> simply a bug fix...

The categories are quite fuzzy, but that also means that getting it
100% right is not that important :)

Personally I'd lean towards bug fix, since it's something that users
might reasonably have expected to work even before now; regardless of
which specific bucket you decide to throw this in,

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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


[libvirt] [PATCH] news: Add entry for soft reset support in Xen

2018-11-08 Thread Jim Fehlig
Signed-off-by: Jim Fehlig 
---

I'm torn on whether this is a new feature, improvement, or even
simply a bug fix...

 docs/news.xml | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 9d98c34df2..e1d10cf7f1 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -35,6 +35,17 @@
 
   
 
+  
+
+  Xen: Add support for soft reset shutdown event
+
+
+  The pvops Linux kernel uses soft reset to handle the crash
+  machine operation. The libxl driver now supports the soft
+  reset shutdown event, allowing proper crash handling of
+  pvops-based HVM domains.
+
+  
 
 
 
-- 
2.18.0

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


Re: [libvirt] [PATCH] libvirt: add daemon itself as shutdown reason

2018-11-08 Thread John Ferlan



On 11/8/18 8:10 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 08.11.2018 15:58, John Ferlan wrote:
>>
>>
>> On 11/8/18 2:03 AM, Nikolay Shirokovskiy wrote:
>>>
>>>
>>> On 07.11.2018 17:29, John Ferlan wrote:


 On 10/8/18 7:21 AM, Nikolay Shirokovskiy wrote:
> Let's introduce shutdown reason "daemon" which means we have to
> kill running domain ourselves as the best action we can take at
> that moment. Failure to pick up domain on daemon restart is
> one example of such case. Using reason "crashed" is a bit misleading
> as it is used when qemu is actually crashed or qemu destroyed on
> guest panic as result of user choice that is the problem was
> in qemu/guest itself. So I propose to use "crashed" only for
> qemu side issues and introduce "daemon" when we have to kill the qemu
> for good.
>
> There is another example where "daemon" will be useful. If we can
> not reboot domain we kill it and got "crashed" reason now. Looks
> like good candidate for "daemon" reason.
>
> Signed-off-by: Nikolay Shirokovskiy 
> ---
>  include/libvirt/libvirt-domain.h |  1 +
>  src/conf/domain_conf.c   |  3 ++-
>  src/qemu/qemu_process.c  | 11 ---
>  tools/virsh-domain-monitor.c |  3 ++-
>  4 files changed, 9 insertions(+), 9 deletions(-)
>
>>
>> [...]
>>
 @@ -8045,7 +8049,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
   * is no thread that could be doing anything else with the same
 domain
   * object.
   */
 -qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED,
 +qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_DAEMON,
  QEMU_ASYNC_JOB_NONE, 0);
>>>
>>> I think we need to report VIR_DOMAIN_SHUTOFF_UNKNOWN here. This is similar 
>>> to 
>>> qemuProcessReconnect before we try to reconnect. May be this hunk need to 
>>> be 
>>> placed in distinct patch therefore.
>>>
>>
>> Perhaps keeping "FAILED" would be better - that way (and I can add this
>> to the qemuProcessReconnect description):
>>
>>  * Failure to reconnect will generate the following reasons:
>>  *VIR_DOMAIN_SHUTOFF_FAILED  => Tried to create this thread, but failed
>>  *VIR_DOMAIN_SHUTOFF_UNKNOWN => Thread started, but failed before 
>> reconnect
>>  *  to the monitor
>>  *VIR_DOMAIN_SHUTOFF_CRASHED => Attempted to reconnect to the monitor, 
>> but
>>  *  failed to open, assume domain crash
>>  *VIR_DOMAIN_SHUTOFF_DAEMON  => Reconnected to the monitor, but decided
>>  *  afterwards we needed to stop the process
>>
>> Look good?
> 
> Unfortunately meaning of each is fixed in API:
> 
> VIR_DOMAIN_SHUTOFF_UNKNOWN = 0, /* the reason is unknown */   
>
> 
> VIR_DOMAIN_SHUTOFF_CRASHED = 3, /* domain crashed */  
>
> 
> VIR_DOMAIN_SHUTOFF_FAILED = 6,  /* domain failed to start */
> 
> So this is definetly not FAILED. I guess we can't change meaning now also, so 
> we have to use UNKNOWN and explaining reasons again is qemuProcessReconnect is
> needless.
> 
> By the way how from reconnection POV failing to spawn a thread differ from any
> error in qemuProcessReconnect before monitor connectiton.
> 

True, but let's walk through going through history

The FAILED reason was introduced in commit b046c55d4 (v0.9.2).

Then, commit d38897a5d (v0.9.5) introduced qemuProcessReconnectHelper to
run qemuProcessReconnect in a thread. This commit used FAILED for both
APIs because that kept the same reason.

Eventually commit bda2f17d7 (v0.9.12) changed FAILED to CRASHED and
UNKNOWN based on whether the domain was started w/ -no-shutdown. With
this commit, failure to start a thread would still use FAILED.

That stayed that way until commit fe35b1ad6 (v4.3.0) in inadvertently
removed UNKNOWN while removing the capability used to determine whether
-no-shutdown was used. This left CRASHED as the only reason.

That was repaired as a result of this patch by commit 296e05b54 (4.10.0).

So now the proposal which seems fair to me is to create a category
DAEMON which reduces the UNKNOWN window to better describe why we're
stopping the domain after reconnection.

So, going back to FAILED in qemuProcessReconnectHelper IMO is the right
thing to do at this point. If some patch comes along afterwards to
changed FAILED to UNKNOWN it can be debated then.

So, I'll restore the original FAILED reason, but not document the
various values since the reader of the code could figure it out.

John

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


Re: [libvirt] [PATCH] libvirt: add daemon itself as shutdown reason

2018-11-08 Thread Nikolay Shirokovskiy



On 08.11.2018 15:58, John Ferlan wrote:
> 
> 
> On 11/8/18 2:03 AM, Nikolay Shirokovskiy wrote:
>>
>>
>> On 07.11.2018 17:29, John Ferlan wrote:
>>>
>>>
>>> On 10/8/18 7:21 AM, Nikolay Shirokovskiy wrote:
 Let's introduce shutdown reason "daemon" which means we have to
 kill running domain ourselves as the best action we can take at
 that moment. Failure to pick up domain on daemon restart is
 one example of such case. Using reason "crashed" is a bit misleading
 as it is used when qemu is actually crashed or qemu destroyed on
 guest panic as result of user choice that is the problem was
 in qemu/guest itself. So I propose to use "crashed" only for
 qemu side issues and introduce "daemon" when we have to kill the qemu
 for good.

 There is another example where "daemon" will be useful. If we can
 not reboot domain we kill it and got "crashed" reason now. Looks
 like good candidate for "daemon" reason.

 Signed-off-by: Nikolay Shirokovskiy 
 ---
  include/libvirt/libvirt-domain.h |  1 +
  src/conf/domain_conf.c   |  3 ++-
  src/qemu/qemu_process.c  | 11 ---
  tools/virsh-domain-monitor.c |  3 ++-
  4 files changed, 9 insertions(+), 9 deletions(-)

> 
> [...]
> 
>>> @@ -8045,7 +8049,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
>>>   * is no thread that could be doing anything else with the same
>>> domain
>>>   * object.
>>>   */
>>> -qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED,
>>> +qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_DAEMON,
>>>  QEMU_ASYNC_JOB_NONE, 0);
>>
>> I think we need to report VIR_DOMAIN_SHUTOFF_UNKNOWN here. This is similar 
>> to 
>> qemuProcessReconnect before we try to reconnect. May be this hunk need to be 
>> placed in distinct patch therefore.
>>
> 
> Perhaps keeping "FAILED" would be better - that way (and I can add this
> to the qemuProcessReconnect description):
> 
>  * Failure to reconnect will generate the following reasons:
>  *VIR_DOMAIN_SHUTOFF_FAILED  => Tried to create this thread, but failed
>  *VIR_DOMAIN_SHUTOFF_UNKNOWN => Thread started, but failed before 
> reconnect
>  *  to the monitor
>  *VIR_DOMAIN_SHUTOFF_CRASHED => Attempted to reconnect to the monitor, but
>  *  failed to open, assume domain crash
>  *VIR_DOMAIN_SHUTOFF_DAEMON  => Reconnected to the monitor, but decided
>  *  afterwards we needed to stop the process
> 
> Look good?

Unfortunately meaning of each is fixed in API:

VIR_DOMAIN_SHUTOFF_UNKNOWN = 0, /* the reason is unknown */ 
 

VIR_DOMAIN_SHUTOFF_CRASHED = 3, /* domain crashed */
 

VIR_DOMAIN_SHUTOFF_FAILED = 6,  /* domain failed to start */

So this is definetly not FAILED. I guess we can't change meaning now also, so 
we have to use UNKNOWN and explaining reasons again is qemuProcessReconnect is
needless.

By the way how from reconnection POV failing to spawn a thread differ from any
error in qemuProcessReconnect before monitor connectiton.

Nikolay

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


Re: [libvirt] [PATCH v2 0/3] Add "memfd" memory backing type

2018-11-08 Thread Marc-André Lureau
Hi
On Thu, Nov 8, 2018 at 4:40 PM Marc-André Lureau
 wrote:
>
> Hi
>
> On Mon, Sep 17, 2018 at 5:15 PM  wrote:
> >
> > From: Marc-André Lureau 
> >
> > Hi,
> >
> > This is an alternative series from "[PATCH 0/5] Use memfd if
> > possible". Instead of automatically using memfd for anonymous memory
> > when available (as suggested by Daniel), it introduces the "memfd"
> > memory backing type.
> >
> > Although using memfd transparently when possible is a good idea, it is
> > a source of various complications for migration & save/restore. This
> > could eventually be challenged in a different series.
> >
> > The first two patches have been modified and reviewed by John
> > Ferlan. Hopefully they can be merged early, regardless of the last
> > patch outcome, to avoid the painful rebase conflicts due to
> > capabilities checks introduction.
>
> ping,
>
> There is a minor qemu caps conflict now, and version should be updated.
>

fwiw, I pushed an updated tree
https://github.com/elmarco/libvirt/commits/memfd

waiting for some feedback on v2 before resending.

> thanks
>
> >
> > Thanks
> >
> > Marc-André Lureau (3):
> >   qemu: add memory-backend-memfd capability check
> >   qemu: check memory-backend-memfd.hugetlb capability
> >   qemu: add memfd source type
> >
> >  docs/formatdomain.html.in |   9 +-
> >  docs/schemas/domaincommon.rng |   1 +
> >  src/conf/domain_conf.c|   3 +-
> >  src/conf/domain_conf.h|   1 +
> >  src/qemu/qemu_capabilities.c  |  10 ++
> >  src/qemu/qemu_capabilities.h  |   2 +
> >  src/qemu/qemu_command.c   |  69 +++
> >  src/qemu/qemu_domain.c|  12 +-
> >  .../caps_2.12.0.aarch64.replies   |  94 ---
> >  .../caps_2.12.0.aarch64.xml   |   4 +-
> >  .../caps_2.12.0.ppc64.replies |  90 +++---
> >  .../caps_2.12.0.ppc64.xml |   4 +-
> >  .../caps_2.12.0.s390x.replies |  98 
> >  .../caps_2.12.0.s390x.xml |   4 +-
> >  .../caps_2.12.0.x86_64.replies| 110 +-
> >  .../caps_2.12.0.x86_64.xml|   4 +-
> >  .../caps_3.0.0.ppc64.replies  |  90 +++---
> >  .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |   4 +-
> >  .../caps_3.0.0.riscv32.replies|  86 +++---
> >  .../caps_3.0.0.riscv32.xml|   2 +
> >  .../caps_3.0.0.riscv64.replies|  86 +++---
> >  .../caps_3.0.0.riscv64.xml|   2 +
> >  .../caps_3.0.0.x86_64.replies | 110 +-
> >  .../caps_3.0.0.x86_64.xml |   4 +-
> >  .../memfd-memory-numa.x86_64-latest.args  |  34 ++
> >  tests/qemuxml2argvdata/memfd-memory-numa.xml  |  36 ++
> >  tests/qemuxml2argvtest.c  |   2 +
> >  27 files changed, 788 insertions(+), 183 deletions(-)
> >  create mode 100644 
> > tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
> >  create mode 100644 tests/qemuxml2argvdata/memfd-memory-numa.xml
> >
> > --
> > 2.19.0
> >
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
>
>
>
> --
> Marc-André Lureau



-- 
Marc-André Lureau

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

[libvirt] [PATCH v2 0/4] add disk driver metadata_cache_size option

2018-11-08 Thread Nikolay Shirokovskiy
1st docs patch may need tweaks if merged now (if -blockdev will not be
available in 4.10)

diff from v1 [1]
===
- support only -blockdev configurations
- add 'default' value for the attribute
- set QEMU_CAPS_QCOW2_L2_CACHE_SIZE only for recent qemu versions (see 2nd 
patch)
- factor out API changes to distinct patch
- fix xml2argv test to blockdev configuration
- factor out xml2argv test to distinct patch as it is not yet passing
- other misc changes

[1] https://www.redhat.com/archives/libvir-list/2018-November/msg00016.html

Nikolay Shirokovskiy (4):
  xml: add disk driver metadata_cache_size option
  qemu: caps: add QEMU_CAPS_QCOW2_L2_CACHE_SIZE
  qemu: support metadata-cache-size for blockdev
  DO NOT APPLY: add xml2argv test for metadata_cache_size

 docs/formatdomain.html.in  |  8 
 docs/schemas/domaincommon.rng  | 11 +
 src/conf/domain_conf.c | 17 
 src/conf/domain_conf.h |  9 
 src/qemu/qemu_block.c  |  5 ++-
 src/qemu/qemu_capabilities.c   |  5 +++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 23 +++
 src/qemu/qemu_domain.c |  2 +
 src/util/virstoragefile.c  |  1 +
 src/util/virstoragefile.h  |  1 +
 .../qemuxml2argvdata/disk-metadata_cache_size.args | 39 ++
 .../qemuxml2argvdata/disk-metadata_cache_size.xml  | 42 +++
 tests/qemuxml2argvtest.c   |  2 +
 .../disk-metadata_cache_size.xml   | 48 ++
 tests/qemuxml2xmltest.c|  2 +
 16 files changed, 215 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.args
 create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.xml
 create mode 100644 tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml

-- 
1.8.3.1

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


[libvirt] [PATCH v2 4/4] DO NOT APPLY: add xml2argv test for metadata_cache_size

2018-11-08 Thread Nikolay Shirokovskiy
This needs next:
- turning QEMU_CAPS_BLOCKDEV on
- adding caps data for not yet released qemu 3.1

Signed-off-by: Nikolay Shirokovskiy 
---
 .../qemuxml2argvdata/disk-metadata_cache_size.args | 39 ++
 tests/qemuxml2argvtest.c   |  2 ++
 2 files changed, 41 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.args

diff --git a/tests/qemuxml2argvdata/disk-metadata_cache_size.args 
b/tests/qemuxml2argvdata/disk-metadata_cache_size.args
new file mode 100644
index 000..ec90d2f
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-metadata_cache_size.args
@@ -0,0 +1,39 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name test \
+-S \
+-machine pc-0.13,accel=tcg,usb=off,dump-guest-core=off \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 468404ad-d49c-40f2-9e14-02294f9c1be3 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot menu=on \
+-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 \
+-usb \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/f14.img",\
+"node-name":"libvirt-2-storage","read-only":false,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"qcow2",\
+"l2-cache-size":9223372036854775807,"file":"libvirt-2-storage"}' \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=libvirt-2-format,\
+id=virtio-disk0,bootindex=2 \
+-blockdev '{"driver":"file",\
+"filename":"/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso",\
+"node-name":"libvirt-1-storage","read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw",\
+"file":"libvirt-1-storage"}' \
+-device ide-drive,bus=ide.1,unit=0,drive=libvirt-1-format,id=ide0-1-0,\
+bootindex=1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 39a7f1f..6f42b09 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3044,6 +3044,8 @@ mymain(void)
 DO_TEST_CAPS_ARCH_LATEST("x86_64-pc-headless", "x86_64");
 DO_TEST_CAPS_ARCH_LATEST("x86_64-q35-headless", "x86_64");
 
+DO_TEST_CAPS_LATEST("disk-metadata_cache_size");
+
 if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
 virFileDeleteTree(fakerootdir);
 
-- 
1.8.3.1

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


[libvirt] [PATCH v2 3/4] qemu: support metadata-cache-size for blockdev

2018-11-08 Thread Nikolay Shirokovskiy
Just set l2-cache-size to INT64_MAX for all format nodes of
qcow2 type in block node graph.

-drive configuration is not supported because we can not
set l2 cache size down the backing chain in this case.

Note that imlementation sets l2-cache-size and not cache-size.
Unfortunately at time of this patch setting cache-size to INT64_MAX
fails and as guest performance depends only on l2 cache size
and not refcount cache size (which is documented in recent qemu)
we can set l2 directly.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/qemu/qemu_block.c |  5 -
 src/qemu/qemu_command.c   | 23 +++
 src/qemu/qemu_domain.c|  2 ++
 src/util/virstoragefile.c |  1 +
 src/util/virstoragefile.h |  1 +
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 5321dda..8771cc1 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1322,7 +1322,6 @@ 
qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src,
  * 'pass-discard-snapshot'
  * 'pass-discard-other'
  * 'overlap-check'
- * 'l2-cache-size'
  * 'l2-cache-entry-size'
  * 'refcount-cache-size'
  * 'cache-clean-interval'
@@ -1331,6 +1330,10 @@ 
qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src,
 if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow2", props) < 
0)
 return -1;
 
+if (src->metadata_cache_size == 
VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM &&
+virJSONValueObjectAdd(props, "I:l2-cache-size", INT64_MAX, NULL) < 0)
+return -1;
+
 return 0;
 }
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f59cbf5..12b2c8d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1330,6 +1330,20 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk,
 return -1;
 }
 
+if (disk->metadata_cache_size) {
+if (disk->src->format != VIR_STORAGE_FILE_QCOW2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("metadata_cache_size can only be set for qcow2 
disks"));
+return -1;
+}
+
+if (disk->metadata_cache_size != 
VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("metadata_cache_size can only be set to 
'maximum'"));
+return -1;
+}
+}
+
 if (qemuCaps) {
 if (disk->serial &&
 disk->bus == VIR_DOMAIN_DISK_BUS_SCSI &&
@@ -1353,6 +1367,15 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk,
_("detect_zeroes is not supported by this QEMU 
binary"));
 return -1;
 }
+
+if (disk->metadata_cache_size &&
+!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) &&
+  virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_L2_CACHE_SIZE))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("setting metadata_cache_size is not supported by "
+ "this QEMU binary"));
+return -1;
+}
 }
 
 if (disk->serial &&
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 045a7b4..23d9348 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9074,6 +9074,7 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
 /* "snapshot" is a libvirt internal field and thus can be changed */
 /* startupPolicy is allowed to be updated. Therefore not checked here. */
 CHECK_EQ(transient, "transient", true);
+CHECK_EQ(metadata_cache_size, "metadata_cache_size", true);
 
 /* Note: For some address types the address auto generation for
  * @disk has still not happened at this point (e.g. driver
@@ -13244,6 +13245,7 @@ qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr 
disk,
 src->iomode = disk->iomode;
 src->cachemode = disk->cachemode;
 src->discard = disk->discard;
+src->metadata_cache_size = disk->metadata_cache_size;
 
 if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
 src->floppyimg = true;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 94c32d8..9089e2f 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2210,6 +2210,7 @@ virStorageSourceCopy(const virStorageSource *src,
 ret->cachemode = src->cachemode;
 ret->discard = src->discard;
 ret->detect_zeroes = src->detect_zeroes;
+ret->metadata_cache_size = src->metadata_cache_size;
 
 /* storage driver metadata are not copied */
 ret->drv = NULL;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3ff6c4f..8b57399 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -331,6 +331,7 @@ struct _virStorageSource {
 int cachemode; /* enum virDomainDiskCache */
 int discard; /* enum virDomainDiskDiscard */
 int detect_zeroes; /* enum virDomainDiskDetectZeroes */
+int 

[libvirt] [PATCH v2 1/4] xml: add disk driver metadata_cache_size option

2018-11-08 Thread Nikolay Shirokovskiy
Signed-off-by: Nikolay Shirokovskiy 
---
 docs/formatdomain.html.in  |  8 
 docs/schemas/domaincommon.rng  | 11 +
 src/conf/domain_conf.c | 17 
 src/conf/domain_conf.h |  9 
 .../qemuxml2argvdata/disk-metadata_cache_size.xml  | 42 +++
 .../disk-metadata_cache_size.xml   | 48 ++
 tests/qemuxml2xmltest.c|  2 +
 7 files changed, 137 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.xml
 create mode 100644 tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 269741a..1d186ab 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3556,6 +3556,14 @@
 virt queues for virtio-blk. (Since 
3.9.0)
   
   
+The optional metadata_cache_size attribute specifies
+metadata cache size policy, possible values are "default" and 
"maximum".
+"default" leaves setting cache size to hypervisor, "maximum" makes
+cache size large enough to keep all metadata, this will help if 
workload
+needs access to whole disk all the time. (Since
+4.10.0, QEMU 3.1)
+  
+  
   For virtio disks,
   Virtio-specific options can also be
   set. (Since 3.5.0)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b9ac5df..3e406fc 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1990,6 +1990,9 @@
 
   
   
+
+  
+  
 
   
 
@@ -2090,6 +2093,14 @@
   
 
   
+  
+
+  
+default
+maximum
+  
+
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 237540b..b2185f8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -885,6 +885,11 @@ VIR_ENUM_IMPL(virDomainDiskDetectZeroes, 
VIR_DOMAIN_DISK_DETECT_ZEROES_LAST,
   "on",
   "unmap")
 
+VIR_ENUM_IMPL(virDomainDiskMetadataCacheSize,
+  VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_LAST,
+  "default",
+  "maximum")
+
 VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST,
   "none",
   "yes",
@@ -9375,6 +9380,14 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def,
 }
 VIR_FREE(tmp);
 
+if ((tmp = virXMLPropString(cur, "metadata_cache_size")) &&
+(def->metadata_cache_size = 
virDomainDiskMetadataCacheSizeTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown driver metadata_cache_size value '%s'"), 
tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);
+
 ret = 0;
 
  cleanup:
@@ -23902,6 +23915,10 @@ virDomainDiskDefFormatDriver(virBufferPtr buf,
 if (disk->queues)
 virBufferAsprintf(, " queues='%u'", disk->queues);
 
+if (disk->metadata_cache_size)
+virBufferAsprintf(, " metadata_cache_size='%s'",
+  
virDomainDiskMetadataCacheSizeTypeToString(disk->metadata_cache_size));
+
 virDomainVirtioOptionsFormat(, disk->virtio);
 
 ret = virXMLFormatElement(buf, "driver", , NULL);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e30a4b2..b155058 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -568,6 +568,13 @@ typedef enum {
 VIR_DOMAIN_DISK_DETECT_ZEROES_LAST
 } virDomainDiskDetectZeroes;
 
+typedef enum {
+VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_DEFAULT = 0,
+VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM,
+
+VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_LAST
+} virDomainDiskMetadataCacheSize;
+
 typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo;
 struct _virDomainBlockIoTuneInfo {
 unsigned long long total_bytes_sec;
@@ -672,6 +679,7 @@ struct _virDomainDiskDef {
 int discard; /* enum virDomainDiskDiscard */
 unsigned int iothread; /* unused = 0, > 0 specific thread # */
 int detect_zeroes; /* enum virDomainDiskDetectZeroes */
+int metadata_cache_size; /* enum virDomainDiskMetadataCacheSize */
 char *domain_name; /* backend domain name */
 unsigned int queues;
 virDomainVirtioOptionsPtr virtio;
@@ -3388,6 +3396,7 @@ VIR_ENUM_DECL(virDomainDeviceSGIO)
 VIR_ENUM_DECL(virDomainDiskTray)
 VIR_ENUM_DECL(virDomainDiskDiscard)
 VIR_ENUM_DECL(virDomainDiskDetectZeroes)
+VIR_ENUM_DECL(virDomainDiskMetadataCacheSize)
 VIR_ENUM_DECL(virDomainDiskMirrorState)
 VIR_ENUM_DECL(virDomainController)
 VIR_ENUM_DECL(virDomainControllerModelPCI)
diff --git a/tests/qemuxml2argvdata/disk-metadata_cache_size.xml 
b/tests/qemuxml2argvdata/disk-metadata_cache_size.xml
new file mode 100644
index 000..8ac2599
--- /dev/null
+++ 

[libvirt] [PATCH v2 2/4] qemu: caps: add QEMU_CAPS_QCOW2_L2_CACHE_SIZE

2018-11-08 Thread Nikolay Shirokovskiy
For qemu capable of setting l2-cache-size for qcow2 images
to INT64_MAX and semantics of upper limit on l2 cache
size. We can only check this by qemu version (3.1.0) now.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/qemu/qemu_capabilities.c | 5 +
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ca5af3..49a3b60 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -509,6 +509,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "vfio-pci.display",
   "blockdev",
   "vfio-ap",
+  "qcow2.l2-cache-size",
 );
 
 
@@ -4117,6 +4118,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
 }
 
+/* l2-cache-size before 3001000 does not accept INT64_MAX */
+if (qemuCaps->version >= 3001000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_QCOW2_L2_CACHE_SIZE);
+
 if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
 goto cleanup;
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6bb9a2c..bb2ac17 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -493,6 +493,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */
 QEMU_CAPS_BLOCKDEV, /* -blockdev and blockdev-add are supported */
 QEMU_CAPS_DEVICE_VFIO_AP, /* -device vfio-ap */
+QEMU_CAPS_QCOW2_L2_CACHE_SIZE, /* -blockdev supports l2-cache-size with 
INT64_MAX value */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
1.8.3.1

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


Re: [libvirt] [PATCH] spec: Drop support for Fedora 27

2018-11-08 Thread Michal Privoznik
On 11/08/2018 01:12 PM, Andrea Bolognani wrote:
> In accordance with our platform support policy, now that
> Fedora 29 is out we no longer support building on Fedora 27.
> 
> This allows us to remove a few version checks.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  libvirt.spec.in | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

ACK

Michal

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


Re: [libvirt] [PATCH] spec: Drop support for Fedora 27

2018-11-08 Thread Erik Skultety
On Thu, Nov 08, 2018 at 01:12:44PM +0100, Andrea Bolognani wrote:
> In accordance with our platform support policy, now that
> Fedora 29 is out we no longer support building on Fedora 27.
>
> This allows us to remove a few version checks.
>
> Signed-off-by: Andrea Bolognani 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH] libvirt: add daemon itself as shutdown reason

2018-11-08 Thread John Ferlan



On 11/8/18 2:03 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 07.11.2018 17:29, John Ferlan wrote:
>>
>>
>> On 10/8/18 7:21 AM, Nikolay Shirokovskiy wrote:
>>> Let's introduce shutdown reason "daemon" which means we have to
>>> kill running domain ourselves as the best action we can take at
>>> that moment. Failure to pick up domain on daemon restart is
>>> one example of such case. Using reason "crashed" is a bit misleading
>>> as it is used when qemu is actually crashed or qemu destroyed on
>>> guest panic as result of user choice that is the problem was
>>> in qemu/guest itself. So I propose to use "crashed" only for
>>> qemu side issues and introduce "daemon" when we have to kill the qemu
>>> for good.
>>>
>>> There is another example where "daemon" will be useful. If we can
>>> not reboot domain we kill it and got "crashed" reason now. Looks
>>> like good candidate for "daemon" reason.
>>>
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  include/libvirt/libvirt-domain.h |  1 +
>>>  src/conf/domain_conf.c   |  3 ++-
>>>  src/qemu/qemu_process.c  | 11 ---
>>>  tools/virsh-domain-monitor.c |  3 ++-
>>>  4 files changed, 9 insertions(+), 9 deletions(-)
>>>

[...]

>> @@ -8045,7 +8049,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
>>   * is no thread that could be doing anything else with the same
>> domain
>>   * object.
>>   */
>> -qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED,
>> +qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_DAEMON,
>>  QEMU_ASYNC_JOB_NONE, 0);
> 
> I think we need to report VIR_DOMAIN_SHUTOFF_UNKNOWN here. This is similar to 
> qemuProcessReconnect before we try to reconnect. May be this hunk need to be 
> placed in distinct patch therefore.
> 

Perhaps keeping "FAILED" would be better - that way (and I can add this
to the qemuProcessReconnect description):

 * Failure to reconnect will generate the following reasons:
 *VIR_DOMAIN_SHUTOFF_FAILED  => Tried to create this thread, but failed
 *VIR_DOMAIN_SHUTOFF_UNKNOWN => Thread started, but failed before reconnect
 *  to the monitor
 *VIR_DOMAIN_SHUTOFF_CRASHED => Attempted to reconnect to the monitor, but
 *  failed to open, assume domain crash
 *VIR_DOMAIN_SHUTOFF_DAEMON  => Reconnected to the monitor, but decided
 *  afterwards we needed to stop the process

Look good?

Tks -

John


[...]

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


Re: [libvirt] [PATCH v2 0/3] Add "memfd" memory backing type

2018-11-08 Thread Marc-André Lureau
Hi

On Mon, Sep 17, 2018 at 5:15 PM  wrote:
>
> From: Marc-André Lureau 
>
> Hi,
>
> This is an alternative series from "[PATCH 0/5] Use memfd if
> possible". Instead of automatically using memfd for anonymous memory
> when available (as suggested by Daniel), it introduces the "memfd"
> memory backing type.
>
> Although using memfd transparently when possible is a good idea, it is
> a source of various complications for migration & save/restore. This
> could eventually be challenged in a different series.
>
> The first two patches have been modified and reviewed by John
> Ferlan. Hopefully they can be merged early, regardless of the last
> patch outcome, to avoid the painful rebase conflicts due to
> capabilities checks introduction.

ping,

There is a minor qemu caps conflict now, and version should be updated.

thanks

>
> Thanks
>
> Marc-André Lureau (3):
>   qemu: add memory-backend-memfd capability check
>   qemu: check memory-backend-memfd.hugetlb capability
>   qemu: add memfd source type
>
>  docs/formatdomain.html.in |   9 +-
>  docs/schemas/domaincommon.rng |   1 +
>  src/conf/domain_conf.c|   3 +-
>  src/conf/domain_conf.h|   1 +
>  src/qemu/qemu_capabilities.c  |  10 ++
>  src/qemu/qemu_capabilities.h  |   2 +
>  src/qemu/qemu_command.c   |  69 +++
>  src/qemu/qemu_domain.c|  12 +-
>  .../caps_2.12.0.aarch64.replies   |  94 ---
>  .../caps_2.12.0.aarch64.xml   |   4 +-
>  .../caps_2.12.0.ppc64.replies |  90 +++---
>  .../caps_2.12.0.ppc64.xml |   4 +-
>  .../caps_2.12.0.s390x.replies |  98 
>  .../caps_2.12.0.s390x.xml |   4 +-
>  .../caps_2.12.0.x86_64.replies| 110 +-
>  .../caps_2.12.0.x86_64.xml|   4 +-
>  .../caps_3.0.0.ppc64.replies  |  90 +++---
>  .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |   4 +-
>  .../caps_3.0.0.riscv32.replies|  86 +++---
>  .../caps_3.0.0.riscv32.xml|   2 +
>  .../caps_3.0.0.riscv64.replies|  86 +++---
>  .../caps_3.0.0.riscv64.xml|   2 +
>  .../caps_3.0.0.x86_64.replies | 110 +-
>  .../caps_3.0.0.x86_64.xml |   4 +-
>  .../memfd-memory-numa.x86_64-latest.args  |  34 ++
>  tests/qemuxml2argvdata/memfd-memory-numa.xml  |  36 ++
>  tests/qemuxml2argvtest.c  |   2 +
>  27 files changed, 788 insertions(+), 183 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/memfd-memory-numa.xml
>
> --
> 2.19.0
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list



-- 
Marc-André Lureau

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

[libvirt] [PATCH] spec: Drop support for Fedora 27

2018-11-08 Thread Andrea Bolognani
In accordance with our platform support policy, now that
Fedora 29 is out we no longer support building on Fedora 27.

This allows us to remove a few version checks.

Signed-off-by: Andrea Bolognani 
---
 libvirt.spec.in | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 90daefe6f1..71cd45c603 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -4,7 +4,7 @@
 # that's still supported by the vendor. It may work on other distros
 # or versions, but no effort will be made to ensure that going forward.
 %define min_rhel 7
-%define min_fedora 27
+%define min_fedora 28
 
 %if (0%{?fedora} && 0%{?fedora} >= %{min_fedora}) || (0%{?rhel} && 0%{?rhel} 
>= %{min_rhel})
 %define supported_platform 1
@@ -72,7 +72,7 @@
 %endif
 
 # We need a recent enough libiscsi (>= 1.18.0)
-%if 0%{?fedora} >= 28 || 0%{?rhel} > 7
+%if 0%{?fedora} || 0%{?rhel} > 7
 %define with_storage_iscsi_direct 0%{!?_without_storage_iscsi_direct:1}
 %else
 %define with_storage_iscsi_direct 0
@@ -258,7 +258,7 @@ BuildRequires: /usr/bin/pod2man
 %endif
 BuildRequires: gcc
 BuildRequires: git
-%if 0%{?fedora} >= 27 || 0%{?rhel} > 7
+%if 0%{?fedora} || 0%{?rhel} > 7
 BuildRequires: perl-interpreter
 %else
 BuildRequires: perl
@@ -386,7 +386,7 @@ BuildRequires: wireshark-devel >= 2.1.0
 BuildRequires: libssh-devel >= 0.7.0
 %endif
 
-%if 0%{?fedora} > 27 || 0%{?rhel} > 7
+%if 0%{?fedora} || 0%{?rhel} > 7
 BuildRequires: rpcgen
 BuildRequires: libtirpc-devel
 %endif
-- 
2.19.1

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


[libvirt] [PATCH v8 10/14] conf: Allocate/release 'uid' and 'fid' in PCI address

2018-11-08 Thread Yi Min Zhao
This patch adds new functions for reservation, assignment and release
to handle the uid/fid. If the uid/fid is defined in the domain XML,
they will be reserved directly in the collecting phase. If any of them
is not defined, we will find out an available value for them from the
zPCI address hashtable, and reserve them. For the hotplug case there
might not be a zPCI definition. So allocate and reserve uid/fid the
case. Assign if needed and reserve uid/fid for the defined case.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Bjoern Walk 
Reviewed-by: Boris Fiuczynski 
---
 src/conf/device_conf.c |  16 +++
 src/conf/device_conf.h |   3 +
 src/conf/domain_addr.c | 244 +
 src/conf/domain_addr.h |  12 ++
 src/libvirt_private.syms   |   5 +
 src/qemu/qemu_domain_address.c |  59 +++-
 6 files changed, 338 insertions(+), 1 deletion(-)

diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index f9f6b6e38f..44b210d5ec 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -28,6 +28,7 @@
 #include "viruuid.h"
 #include "virbuffer.h"
 #include "device_conf.h"
+#include "domain_addr.h"
 #include "virstring.h"
 
 #define VIR_FROM_THIS VIR_FROM_DEVICE
@@ -215,6 +216,21 @@ virDeviceInfoPCIAddressIsPresent(const virDomainDeviceInfo 
*info)
 }
 
 
+bool
+virDeviceInfoPCIAddressExtensionIsWanted(const virDomainDeviceInfo *info)
+{
+return (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) &&
+   virZPCIDeviceAddressIsEmpty(>addr.pci.zpci);
+}
+
+bool
+virDeviceInfoPCIAddressExtensionIsPresent(const virDomainDeviceInfo *info)
+{
+return (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) &&
+   !virZPCIDeviceAddressIsEmpty(>addr.pci.zpci);
+}
+
+
 int
 virPCIDeviceAddressParseXML(xmlNodePtr node,
 virPCIDeviceAddressPtr addr)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 2366e03607..867b633903 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -198,6 +198,9 @@ bool virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr 
info,
 bool virDeviceInfoPCIAddressIsWanted(const virDomainDeviceInfo *info);
 bool virDeviceInfoPCIAddressIsPresent(const virDomainDeviceInfo *info);
 
+bool virDeviceInfoPCIAddressExtensionIsWanted(const virDomainDeviceInfo *info);
+bool virDeviceInfoPCIAddressExtensionIsPresent(const virDomainDeviceInfo 
*info);
+
 int virPCIDeviceAddressParseXML(xmlNodePtr node,
 virPCIDeviceAddressPtr addr);
 
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 380091f049..3e1d767e4f 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -33,6 +33,238 @@
 
 VIR_LOG_INIT("conf.domain_addr");
 
+static int
+virDomainZPCIAddressReserveId(virHashTablePtr set,
+  unsigned int id,
+  const char *name)
+{
+if (virHashLookup(set, )) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("zPCI %s %o is already reserved"),
+   name, id);
+return -1;
+}
+
+if (virHashAddEntry(set, , (void*)1) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to reserve %s %o"),
+   name, id);
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+virDomainZPCIAddressReserveUid(virHashTablePtr set,
+   virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressReserveId(set, addr->uid, "uid");
+}
+
+
+static int
+virDomainZPCIAddressReserveFid(virHashTablePtr set,
+   virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressReserveId(set, addr->fid, "fid");
+}
+
+
+static int
+virDomainZPCIAddressAssignId(virHashTablePtr set,
+ unsigned int *id,
+ unsigned int min,
+ unsigned int max,
+ const char *name)
+{
+while (virHashLookup(set, )) {
+if (min == max) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("There is no more free %s."),
+   name);
+return -1;
+}
+++min;
+}
+*id = min;
+
+return 0;
+}
+
+
+static int
+virDomainZPCIAddressAssignUid(virHashTablePtr set,
+  virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressAssignId(set, >uid, 1,
+VIR_DOMAIN_DEVICE_ZPCI_MAX_UID, "uid");
+}
+
+
+static int
+virDomainZPCIAddressAssignFid(virHashTablePtr set,
+  virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressAssignId(set, >fid, 0,
+VIR_DOMAIN_DEVICE_ZPCI_MAX_FID, "fid");
+}
+
+
+static void
+virDomainZPCIAddressReleaseId(virHashTablePtr set,
+  unsigned 

[libvirt] [PATCH v8 14/14] news: Update news for PCI address extension attributes

2018-11-08 Thread Yi Min Zhao
Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 docs/news.xml | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 9d98c34df2..4549c0a46f 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -35,6 +35,17 @@
 
   
 
+  
+
+  qemu: Added support for PCI devices on S390
+
+
+  PCI addresses can now include the new zpci element which contains
+  uid (user-defined identifier) and fid (PCI function identifier)
+  attributes and makes the corresponding devices usable by S390
+  guests.
+
+  
 
 
 
-- 
Yi Min

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

[libvirt] [PATCH v8 11/14] qemu: Generate and use zPCI device in QEMU command line

2018-11-08 Thread Yi Min Zhao
Add new functions to generate zPCI command string and append it to
QEMU command line. And the related tests are added.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 src/qemu/qemu_command.c   | 104 ++
 src/qemu/qemu_command.h   |   2 +
 .../disk-virtio-s390-zpci.args|   1 +
 .../hostdev-vfio-zpci-autogenerate.args   |  25 +
 .../hostdev-vfio-zpci-autogenerate.xml|  18 +++
 .../hostdev-vfio-zpci-boundaries.args |  29 +
 .../hostdev-vfio-zpci-boundaries.xml  |  30 +
 .../hostdev-vfio-zpci-multidomain-many.args   |  39 +++
 .../hostdev-vfio-zpci-multidomain-many.xml|  79 +
 tests/qemuxml2argvdata/hostdev-vfio-zpci.args |   2 +
 tests/qemuxml2argvtest.c  |  13 +++
 .../hostdev-vfio-zpci-autogenerate.xml|  34 ++
 .../hostdev-vfio-zpci-boundaries.xml  |  48 
 .../hostdev-vfio-zpci-multidomain-many.xml|  97 
 tests/qemuxml2xmltest.c   |  11 ++
 15 files changed, 532 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml
 create mode 100644 
tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args
 create mode 100644 
tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-boundaries.xml
 create mode 100644 
tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e338d3172e..8efc31f731 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2195,6 +2195,57 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
 return NULL;
 }
 
+char *
+qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAsprintf(,
+  "zpci,uid=%u,fid=%u,target=%s,id=zpci%u",
+  dev->addr.pci.zpci.uid,
+  dev->addr.pci.zpci.fid,
+  dev->alias,
+  dev->addr.pci.zpci.uid);
+
+if (virBufferCheckError() < 0) {
+virBufferFreeAndReset();
+return NULL;
+}
+
+return virBufferContentAndReset();
+}
+
+static int
+qemuCommandAddZPCIDevice(virCommandPtr cmd,
+ virDomainDeviceInfoPtr dev)
+{
+char *devstr = NULL;
+
+virCommandAddArg(cmd, "-device");
+
+if (!(devstr = qemuBuildZPCIDevStr(dev)))
+return -1;
+
+virCommandAddArg(cmd, devstr);
+
+VIR_FREE(devstr);
+return 0;
+}
+
+static int
+qemuCommandAddExtDevice(virCommandPtr cmd,
+virDomainDeviceInfoPtr dev)
+{
+if (dev->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
+dev->addr.pci.extFlags == VIR_PCI_ADDRESS_EXTENSION_NONE) {
+return 0;
+}
+
+if (dev->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI)
+return qemuCommandAddZPCIDevice(cmd, dev);
+
+return 0;
+}
 
 static int
 qemuBuildFloppyCommandLineControllerOptions(virCommandPtr cmd,
@@ -2431,6 +2482,9 @@ qemuBuildDiskCommandLine(virCommandPtr cmd,
 if (!qemuDiskBusNeedsDriveArg(disk->bus)) {
 if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC ||
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (qemuCommandAddExtDevice(cmd, >info) < 0)
+return -1;
+
 virCommandAddArg(cmd, "-device");
 
 if (!(optstr = qemuBuildDiskDeviceStr(def, disk, bootindex,
@@ -2629,6 +2683,9 @@ qemuBuildFSDevCommandLine(virCommandPtr cmd,
 virCommandAddArg(cmd, optstr);
 VIR_FREE(optstr);
 
+if (qemuCommandAddExtDevice(cmd, >info) < 0)
+return -1;
+
 virCommandAddArg(cmd, "-device");
 if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps)))
 return -1;
@@ -3089,6 +3146,11 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
 goto cleanup;
 
 if (devstr) {
+if (qemuCommandAddExtDevice(cmd, >info) < 0) {
+VIR_FREE(devstr);
+goto cleanup;
+}
+
 virCommandAddArg(cmd, "-device");
 virCommandAddArg(cmd, devstr);
 VIR_FREE(devstr);
@@ -3887,6 +3949,9 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd,
 if (!def->watchdog)
 return 0;
 
+if (qemuCommandAddExtDevice(cmd, >watchdog->info) < 0)
+return -1;
+
 virCommandAddArg(cmd, 

[libvirt] [PATCH v8 13/14] docs: Add 'uid' and 'fid' information

2018-11-08 Thread Yi Min Zhao
Update 'Device address' section to describe 'zpci' element and
its two attributes 'uid' and 'fid'.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 docs/formatdomain.html.in | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 269741a690..dd89a95c3c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3925,7 +3925,15 @@
 (since 0.9.7, requires QEMU
 0.13). multifunction defaults to 'off',
 but should be set to 'on' for function 0 of a slot that will
-have multiple functions used.
+have multiple functions used.
+(Since 4.10.0), PCI address extensions
+depending on the architecture are supported. For example, PCI
+addresses for S390 guests will have a zpci child
+element, with two attributes: uid (a hex value
+between 0x0001 and 0x, inclusive), and fid (a
+hex value between 0x and 0x, inclusive) used by
+PCI devices on S390 for User-defined Identifiers and Function
+Identifiers.
 Since 1.3.5, some hypervisor
 drivers may accept an address type='pci'/
 element with no other attributes as an explicit request to
-- 
Yi Min

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

[libvirt] [PATCH v8 08/14] conf: Introduce parser, formatter for uid and fid

2018-11-08 Thread Yi Min Zhao
This patch introduces new XML parser/formatter functions. Uid is
16-bit and non-zero. Fid is 32-bit. They are the two attributes of zpci
which is introduced as PCI address element. Zpci element is parsed and
formatted along with PCI address. And add the related test cases.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 docs/schemas/basictypes.rng   | 27 ++
 docs/schemas/domaincommon.rng |  1 +
 src/conf/device_conf.c| 53 +++
 src/conf/domain_addr.c|  3 ++
 src/conf/domain_conf.c| 12 -
 src/libvirt_private.syms  |  2 +
 src/util/virpci.c | 26 +
 src/util/virpci.h |  6 +++
 .../disk-virtio-s390-zpci.args| 25 +
 .../disk-virtio-s390-zpci.xml | 19 +++
 tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 23 
 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml  | 21 
 tests/qemuxml2argvtest.c  |  7 +++
 .../disk-virtio-s390-zpci.xml | 31 +++
 .../qemuxml2xmloutdata/hostdev-vfio-zpci.xml  | 32 +++
 tests/qemuxml2xmltest.c   |  6 +++
 16 files changed, 293 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args
 create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml
 create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 14a3670e5c..71a6db3bb4 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -65,6 +65,17 @@
   
 
   
+  
+
+  
+(0x)?[0-9a-fA-F]{1,8}
+  
+  
+0
+4294967295
+  
+
+  
 
   
 
@@ -111,6 +122,22 @@
   
 
   
+  
+
+  
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
+  
 
   
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b9ac5df479..d2f5f16266 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5221,6 +5221,7 @@
 pci
   
   
+  
 
 
   
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 98a419f40f..f9f6b6e38f 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -47,6 +47,45 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
   "dimm",
 );
 
+static int
+virZPCIDeviceAddressParseXML(xmlNodePtr node,
+ virPCIDeviceAddressPtr addr)
+{
+virZPCIDeviceAddress def = { 0 };
+char *uid;
+char *fid;
+int ret = -1;
+
+uid = virXMLPropString(node, "uid");
+fid = virXMLPropString(node, "fid");
+
+if (uid &&
+virStrToLong_uip(uid, NULL, 0, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Cannot parse  'uid' attribute"));
+goto cleanup;
+}
+
+if (fid &&
+virStrToLong_uip(fid, NULL, 0, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Cannot parse  'fid' attribute"));
+goto cleanup;
+}
+
+if (!virZPCIDeviceAddressIsEmpty() &&
+!virZPCIDeviceAddressIsValid())
+goto cleanup;
+
+addr->zpci = def;
+ret = 0;
+
+ cleanup:
+VIR_FREE(uid);
+VIR_FREE(fid);
+return ret;
+}
+
 int
 virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst,
 virDomainDeviceInfoPtr src)
@@ -181,6 +220,8 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
 virPCIDeviceAddressPtr addr)
 {
 char *domain, *slot, *bus, *function, *multi;
+xmlNodePtr cur;
+xmlNodePtr zpci = NULL;
 int ret = -1;
 
 memset(addr, 0, sizeof(*addr));
@@ -230,6 +271,18 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
 if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, 
true))
 goto cleanup;
 
+cur = node->children;
+while (cur) {
+if (cur->type == XML_ELEMENT_NODE &&
+virXMLNodeNameEqual(cur, "zpci")) {
+zpci = cur;
+}
+cur = cur->next;
+}
+
+if (zpci && virZPCIDeviceAddressParseXML(zpci, addr) < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 0c00302c13..380091f049 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1040,6 +1040,9 

[libvirt] [PATCH v8 01/14] conf: Add definitions for 'uid' and 'fid' PCI address attributes

2018-11-08 Thread Yi Min Zhao
Add zPCI definitions in preparation of extending the PCI address
with parameters uid (user-defined identifier) and fid (PCI function
identifier).

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 cfg.mk| 1 +
 src/util/virpci.h | 7 +++
 2 files changed, 8 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index d0183c02ff..b108553ca8 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -472,6 +472,7 @@ sc_prohibit_canonicalize_file_name:
 # Insist on correct types for [pug]id.
 sc_correct_id_types:
@prohibit='\<(int|long) *[pug]id\>' \
+   exclude='exempt from syntax-check' \
halt='use pid_t for pid, uid_t for uid, gid_t for gid' \
  $(_sc_search_regexp)
 
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 16c2eded5e..4fd1828e9c 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -37,6 +37,13 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr;
 typedef struct _virPCIDeviceList virPCIDeviceList;
 typedef virPCIDeviceList *virPCIDeviceListPtr;
 
+typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress;
+typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr;
+struct _virZPCIDeviceAddress {
+unsigned int uid; /* exempt from syntax-check */
+unsigned int fid;
+};
+
 struct _virPCIDeviceAddress {
 unsigned int domain;
 unsigned int bus;
-- 
Yi Min

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

[libvirt] [PATCH v8 00/14] PCI passthrough support on s390

2018-11-08 Thread Yi Min Zhao
Abstract

The PCI representation in QEMU has been extended for S390
allowing configuration of zPCI attributes like uid (user-defined
identifier) and fid (PCI function identifier).
The details can be found here:
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html

To support the new zPCI feature of the S390 platform, a new element of
PCI address is introduced. It has two optional attributes, @uid and
@fid. For example:
  


  


  

  

If they are defined by the user, unique values within the guest domain
must be used. If they are not specified and the architecture requires
them, they are automatically generated with non-conflicting values.

zPCI address as an extension of the PCI address are stored in a new
structure 'virZPCIDeviceAddress' which is a member of common PCI
Address structure. Additionally, two hashtables are used for assignment
and reservation of zPCI uid/fid.

In support of extending the PCI address, a new PCI address extension flag is
introduced. This extension flag allows is not only dedicated for the S390
platform but also other architectures needing certain extensions to PCI
address space.

Code Base
=
commit in master:
4f1107614d docs: Enhance polkit documentation to describe secondary connection

Change Log
==
v7->v8:
1. Rebase the code to the newest code in master branch.
2. Update the words regarding version number in docs to 4.10.0.
3. Move the code introducing zpci member from patch 1 to patch 3.

v6->v7:
1. Optimize some functions' names and code logic.
2. Fixup build error.
3. Add negative test case for patch 9.
4. Use virXMLFormatElement() in virDomainDeviceInfoFormat().

v5->v6:
1. Modify zPCI XML definition.
2. Optimize the logic of zPCI address assignment and reservation.
3. Add extension flag into PCI address structure.
4. Update commit messages.

v4->v5:
1. Update the version number.
2. Fixup code style error.
3. Separate qemu code into single patch.
4. Rebase the patches to the new code of master branch.

v3->v4:
1. Update docs.
2. Format code style.
3. Optimize zPCI support check.
4. Move the check of zPCI defined in xml but unsupported by Qemu to
   qemuDomainDeviceDefValidate().
5. Change zpci address member of PCI address struct from pointer to
   instance.
6. Modify zpci address definition principle. Currently the user must
   either define both of uid and fid or not.

v2->v3:
1. Revise code style.
2. Update test cases.
3. Introduce qemuDomainCollectPCIAddressExtension() to collect PCI
   extension addresses.
4. Introduce virDeviceInfoPCIAddressExtensionPresent() to check if zPCI
   address exists.
5. Optimize zPCI address check logic.
6. Optimize passed parameters of zPCI addr alloc/release/reserve functions.
7. Report enum range error in qemuDomainDeviceSupportZPCI().
8. Update commit messages.

v1->v2:
1. Separate test commit and merge testcases into corresponding commits that
   introduce the functionalities firstly.
2. Spare some checks for zpci device.
3. Add vsock and controller support.
4. Add uin32 type schema.
5. Rename zpciuid and zpcifid to zpci_uid and zpci_fid.
6. Always return multibus support on S390.

Yi Min Zhao (14):
  conf: Add definitions for 'uid' and 'fid' PCI address attributes
  qemu: Introduce zPCI capability
  conf: Introduce extension flag and zPCI member for PCI address
  qemu: Enable PCI multi bus for S390 guests
  qemu: Auto add pci-root for s390/s390x guests
  conf: Introduce address caching for PCI extensions
  conf: use virXMLFormatElement() in virDomainDeviceInfoFormat()
  conf: Introduce parser, formatter for uid and fid
  qemu: Add zPCI address definition check
  conf: Allocate/release 'uid' and 'fid' in PCI address
  qemu: Generate and use zPCI device in QEMU command line
  qemu: Add hotpluging support for PCI devices on S390 guests
  docs: Add 'uid' and 'fid' information
  news: Update news for PCI address extension attributes

 cfg.mk|   1 +
 docs/formatdomain.html.in |  10 +-
 docs/news.xml |  11 +
 docs/schemas/basictypes.rng   |  27 ++
 docs/schemas/domaincommon.rng |   1 +
 src/bhyve/bhyve_device.c  |   3 +-
 src/conf/device_conf.c|  69 
 src/conf/device_conf.h|   7 +
 src/conf/domain_addr.c| 340 +-
 src/conf/domain_addr.h|  27 +-
 src/conf/domain_conf.c|  50 ++-
 src/libvirt_private.syms  |   7 +
 src/qemu/qemu_capabilities.c  |   6 +
 src/qemu/qemu_capabilities.h  |   1 +
 src/qemu/qemu_command.c   | 104 ++
 src/qemu/qemu_command.h   |   2 +
 src/qemu/qemu_domain.c|  37 ++
 src/qemu/qemu_domain_address.c| 205 ++-
 

[libvirt] [PATCH v8 09/14] qemu: Add zPCI address definition check

2018-11-08 Thread Yi Min Zhao
We should ensure that QEMU supports zPCI when a zPCI address is defined
in XML and otherwise report an error. This patch introduces a generic
validation function qemuDomainDeviceDefValidateAddress() which calls
qemuDomainDeviceDefValidateZPCIAddress() if address type is PCI address.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Andrea Bolognani 
---
 src/qemu/qemu_domain.c| 36 +++
 .../hostdev-vfio-zpci-wrong-arch.xml  | 34 ++
 tests/qemuxml2argvtest.c  |  2 ++
 3 files changed, 72 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-wrong-arch.xml

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3c1a616b40..327a73a3f7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5800,6 +5800,38 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef 
*input,
 }
 
 
+static int
+qemuDomainDeviceDefValidateZPCIAddress(virDomainDeviceInfoPtr info,
+   virQEMUCapsPtr qemuCaps)
+{
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci) &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   "%s",
+   _("This QEMU binary doesn't support zPCI"));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+qemuDomainDeviceDefValidateAddress(const virDomainDeviceDef *dev,
+   virQEMUCapsPtr qemuCaps)
+{
+virDomainDeviceInfoPtr info;
+
+if (!(info = virDomainDeviceGetInfo((virDomainDeviceDef *)dev)))
+return 0;
+
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+return qemuDomainDeviceDefValidateZPCIAddress(info, qemuCaps);
+
+return 0;
+}
+
+
 static int
 qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
 const virDomainDef *def,
@@ -5813,6 +5845,9 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
 def->emulator)))
 return -1;
 
+if ((ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps)) < 0)
+goto cleanup;
+
 switch ((virDomainDeviceType)dev->type) {
 case VIR_DOMAIN_DEVICE_NET:
 ret = qemuDomainDeviceDefValidateNetwork(dev->data.net);
@@ -5888,6 +5923,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
 break;
 }
 
+ cleanup:
 virObjectUnref(qemuCaps);
 return ret;
 }
diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-wrong-arch.xml 
b/tests/qemuxml2argvdata/hostdev-vfio-zpci-wrong-arch.xml
new file mode 100644
index 00..bfb2f83a3b
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-wrong-arch.xml
@@ -0,0 +1,34 @@
+
+  QEMUGuest2
+  c7a5fdbd-edaf-9466-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+  
+
+
+
+  
+  
+
+  
+  
+
+  
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7b9e6bf2be..09c52f5f38 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1683,6 +1683,8 @@ mymain(void)
 DO_TEST_PARSE_ERROR("hostdev-mdev-display-missing-graphics",
 QEMU_CAPS_DEVICE_VFIO_PCI,
 QEMU_CAPS_VFIO_PCI_DISPLAY);
+DO_TEST_PARSE_ERROR("hostdev-vfio-zpci-wrong-arch",
+QEMU_CAPS_DEVICE_VFIO_PCI);
 DO_TEST("hostdev-vfio-zpci",
 QEMU_CAPS_DEVICE_VFIO_PCI,
 QEMU_CAPS_DEVICE_ZPCI);
-- 
Yi Min

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


[libvirt] [PATCH v8 06/14] conf: Introduce address caching for PCI extensions

2018-11-08 Thread Yi Min Zhao
This patch provides a caching mechanism for the device address
extensions uid and fid on S390. For efficient sparse address allocation,
we introduce two hash tables for uid/fid which hold the address set
information per domain. Also in order to improve performance of
searching available value, we introduce our own callbacks for the two
hashtables. In this way, uid/fid is saved in hash key and hash value
could be any non-NULL pointer due to no operation on hash value. That is
also the reason why we don't introduce hash value free callback.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 src/bhyve/bhyve_device.c   |  3 +-
 src/conf/domain_addr.c | 93 +-
 src/conf/domain_addr.h | 10 +++-
 src/qemu/qemu_domain_address.c |  6 ++-
 4 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
index 207ac6a2dd..a59dfe4519 100644
--- a/src/bhyve/bhyve_device.c
+++ b/src/bhyve/bhyve_device.c
@@ -71,7 +71,8 @@ bhyveDomainPCIAddressSetCreate(virDomainDefPtr def, unsigned 
int nbuses)
 {
 virDomainPCIAddressSetPtr addrs;
 
-if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL)
+if ((addrs = virDomainPCIAddressSetAlloc(nbuses,
+ VIR_PCI_ADDRESS_EXTENSION_NONE)) 
== NULL)
 return NULL;
 
 if (virDomainPCIAddressBusSetModel(>buses[0],
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index e4ed143b76..0c00302c13 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -27,6 +27,7 @@
 #include "virlog.h"
 #include "virstring.h"
 #include "domain_addr.h"
+#include "virhashcode.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -727,8 +728,93 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr 
addrs,
 addrs->buses[addr->bus].slot[addr->slot].functions &= ~(1 << 
addr->function);
 }
 
+
+static uint32_t
+virZPCIAddrKeyCode(const void *name,
+   uint32_t seed)
+{
+unsigned int value = *((unsigned int *)name);
+return virHashCodeGen(, sizeof(value), seed);
+}
+
+
+static bool
+virZPCIAddrKeyEqual(const void *namea,
+const void *nameb)
+{
+return *((unsigned int *)namea) == *((unsigned int *)nameb);
+}
+
+
+static void *
+virZPCIAddrKeyCopy(const void *name)
+{
+unsigned int *copy;
+
+if (VIR_ALLOC(copy) < 0)
+return NULL;
+
+*copy = *((unsigned int *)name);
+return (void *)copy;
+}
+
+
+static void
+virZPCIAddrKeyFree(void *name)
+{
+VIR_FREE(name);
+}
+
+
+static void
+virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
+{
+if (!addrs || !addrs->zpciIds)
+return;
+
+virHashFree(addrs->zpciIds->uids);
+virHashFree(addrs->zpciIds->fids);
+VIR_FREE(addrs->zpciIds);
+}
+
+
+static int
+virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
+ virPCIDeviceAddressExtensionFlags 
extFlags)
+{
+if (extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) {
+if (addrs->zpciIds)
+return 0;
+
+if (VIR_ALLOC(addrs->zpciIds) < 0)
+return -1;
+
+if (!(addrs->zpciIds->uids = virHashCreateFull(10, NULL,
+   virZPCIAddrKeyCode,
+   virZPCIAddrKeyEqual,
+   virZPCIAddrKeyCopy,
+   virZPCIAddrKeyFree)))
+goto error;
+
+if (!(addrs->zpciIds->fids = virHashCreateFull(10, NULL,
+   virZPCIAddrKeyCode,
+   virZPCIAddrKeyEqual,
+   virZPCIAddrKeyCopy,
+   virZPCIAddrKeyFree)))
+goto error;
+}
+
+return 0;
+
+ error:
+virDomainPCIAddressSetExtensionFree(addrs);
+return -1;
+}
+
+
 virDomainPCIAddressSetPtr
-virDomainPCIAddressSetAlloc(unsigned int nbuses)
+virDomainPCIAddressSetAlloc(unsigned int nbuses,
+virPCIDeviceAddressExtensionFlags extFlags)
 {
 virDomainPCIAddressSetPtr addrs;
 
@@ -739,6 +825,10 @@ virDomainPCIAddressSetAlloc(unsigned int nbuses)
 goto error;
 
 addrs->nbuses = nbuses;
+
+if (virDomainPCIAddressSetExtensionAlloc(addrs, extFlags) < 0)
+goto error;
+
 return addrs;
 
  error:
@@ -753,6 +843,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs)
 if (!addrs)
 return;
 
+virDomainPCIAddressSetExtensionFree(addrs);
 VIR_FREE(addrs->buses);
 VIR_FREE(addrs);
 }
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 435b3c5d7f..8d64d6b795 100644
--- 

[libvirt] [PATCH v8 05/14] qemu: Auto add pci-root for s390/s390x guests

2018-11-08 Thread Yi Min Zhao
The pci-root depends on zpci capability. So autogenerate pci-root if
zpci exists.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 src/qemu/qemu_domain.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ba3fff607a..3c1a616b40 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3290,6 +3290,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 case VIR_ARCH_S390X:
 addDefaultUSB = false;
 addPanicDevice = true;
+addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI);
 break;
 
 case VIR_ARCH_SPARC:
-- 
Yi Min

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

[libvirt] [PATCH v8 04/14] qemu: Enable PCI multi bus for S390 guests

2018-11-08 Thread Yi Min Zhao
QEMU on s390 supports PCI multibus since forever.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index eed27f6878..8294e1b4a8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1702,6 +1702,10 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps,
 return false;
 }
 
+/* S390 supports PCI-multibus. */
+if (ARCH_IS_S390(def->os.arch))
+return true;
+
 /* If ARM 'virt' supports PCI, it supports multibus.
  * No extra conditions here for simplicity.
  */
-- 
Yi Min

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

[libvirt] [PATCH v8 07/14] conf: use virXMLFormatElement() in virDomainDeviceInfoFormat()

2018-11-08 Thread Yi Min Zhao
In order to add zPCI child element for PCI address, we update
virDomainDeviceInfoFormat() to format device info by helper function
virXMLFormatElement(). Then we could simply format zPCI address into
child buffer later.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Andrea Bolognani 
---
 src/conf/domain_conf.c | 40 ++--
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 237540bccc..3e89659d3e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6428,6 +6428,8 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
   virDomainDeviceInfoPtr info,
   unsigned int flags)
 {
+virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
+
 if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) {
 virBufferAsprintf(buf, "bootIndex);
 
@@ -6472,13 +6474,13 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
 return;
 
-virBufferAsprintf(buf, "type));
 
 switch ((virDomainDeviceAddressType) info->type) {
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
 if (!virPCIDeviceAddressIsEmpty(>addr.pci)) {
-virBufferAsprintf(buf, " domain='0x%.4x' bus='0x%.2x' "
+virBufferAsprintf(, " domain='0x%.4x' bus='0x%.2x' "
   "slot='0x%.2x' function='0x%.1x'",
   info->addr.pci.domain,
   info->addr.pci.bus,
@@ -6486,13 +6488,13 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
   info->addr.pci.function);
 }
 if (info->addr.pci.multi) {
-   virBufferAsprintf(buf, " multifunction='%s'",
- 
virTristateSwitchTypeToString(info->addr.pci.multi));
+virBufferAsprintf(, " multifunction='%s'",
+  
virTristateSwitchTypeToString(info->addr.pci.multi));
 }
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
-virBufferAsprintf(buf, " controller='%d' bus='%d' target='%d' 
unit='%d'",
+virBufferAsprintf(, " controller='%d' bus='%d' target='%d' 
unit='%d'",
   info->addr.drive.controller,
   info->addr.drive.bus,
   info->addr.drive.target,
@@ -6500,34 +6502,34 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
-virBufferAsprintf(buf, " controller='%d' bus='%d' port='%d'",
+virBufferAsprintf(, " controller='%d' bus='%d' port='%d'",
   info->addr.vioserial.controller,
   info->addr.vioserial.bus,
   info->addr.vioserial.port);
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
-virBufferAsprintf(buf, " controller='%d' slot='%d'",
+virBufferAsprintf(, " controller='%d' slot='%d'",
   info->addr.ccid.controller,
   info->addr.ccid.slot);
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
-virBufferAsprintf(buf, " bus='%d'", info->addr.usb.bus);
+virBufferAsprintf(, " bus='%d'", info->addr.usb.bus);
 if (virDomainUSBAddressPortIsValid(info->addr.usb.port)) {
-virBufferAddLit(buf, " port='");
-virDomainUSBAddressPortFormatBuf(buf, info->addr.usb.port);
-virBufferAddLit(buf, "'");
+virBufferAddLit(, " port='");
+virDomainUSBAddressPortFormatBuf(, info->addr.usb.port);
+virBufferAddLit(, "'");
 }
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
 if (info->addr.spaprvio.has_reg)
-virBufferAsprintf(buf, " reg='0x%llx'", info->addr.spaprvio.reg);
+virBufferAsprintf(, " reg='0x%llx'", 
info->addr.spaprvio.reg);
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
-virBufferAsprintf(buf, " cssid='0x%x' ssid='0x%x' devno='0x%04x'",
+virBufferAsprintf(, " cssid='0x%x' ssid='0x%x' devno='0x%04x'",
   info->addr.ccw.cssid,
   info->addr.ccw.ssid,
   info->addr.ccw.devno);
@@ -6538,15 +6540,15 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
 if (info->addr.isa.iobase > 0)
-virBufferAsprintf(buf, " iobase='0x%x'", info->addr.isa.iobase);
+virBufferAsprintf(, " iobase='0x%x'", 
info->addr.isa.iobase);
 if (info->addr.isa.irq > 0)
-virBufferAsprintf(buf, " irq='0x%x'", info->addr.isa.irq);
+virBufferAsprintf(, " irq='0x%x'", info->addr.isa.irq);
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
-virBufferAsprintf(buf, " slot='%u'", 

[libvirt] [PATCH v8 12/14] qemu: Add hotpluging support for PCI devices on S390 guests

2018-11-08 Thread Yi Min Zhao
This commit adds hotplug support for PCI devices on S390 guests.
There's no need to implement hot unplug for zPCI as QEMU implements
an unplug callback which will unplug both PCI and zPCI device in a
cascaded way.
Currently, the following PCI devices are supported:
  virtio-blk-pci
  virtio-net-pci
  virtio-rng-pci
  virtio-input-host-pci
  virtio-keyboard-pci
  virtio-mouse-pci
  virtio-tablet-pci
  vfio-pci
  SCSIVhost device

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 src/qemu/qemu_hotplug.c | 160 +---
 1 file changed, 151 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0a63741b9e..5f756b7267 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -154,6 +154,80 @@ qemuHotplugPrepareDiskAccess(virQEMUDriverPtr driver,
 }
 
 
+static int
+qemuDomainAttachZPCIDevice(qemuMonitorPtr mon,
+   virDomainDeviceInfoPtr info)
+{
+char *devstr_zpci = NULL;
+int ret = -1;
+
+if (!(devstr_zpci = qemuBuildZPCIDevStr(info)))
+goto cleanup;
+
+if (qemuMonitorAddDevice(mon, devstr_zpci) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(devstr_zpci);
+return ret;
+}
+
+
+static int
+qemuDomainDetachZPCIDevice(qemuMonitorPtr mon,
+   virDomainDeviceInfoPtr info)
+{
+char *zpciAlias = NULL;
+int ret = -1;
+
+if (virAsprintf(, "zpci%d", info->addr.pci.zpci.uid) < 0)
+goto cleanup;
+
+if (qemuMonitorDelDevice(mon, zpciAlias) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(zpciAlias);
+return ret;
+}
+
+
+static int
+qemuDomainAttachExtensionDevice(qemuMonitorPtr mon,
+virDomainDeviceInfoPtr info)
+{
+if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
+info->addr.pci.extFlags == VIR_PCI_ADDRESS_EXTENSION_NONE) {
+return 0;
+}
+
+if (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI)
+return qemuDomainAttachZPCIDevice(mon, info);
+
+return 0;
+}
+
+
+static int
+qemuDomainDetachExtensionDevice(qemuMonitorPtr mon,
+virDomainDeviceInfoPtr info)
+{
+if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
+info->addr.pci.extFlags == VIR_PCI_ADDRESS_EXTENSION_NONE) {
+return 0;
+}
+
+if (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI)
+return qemuDomainDetachZPCIDevice(mon, info);
+
+return 0;
+}
+
+
 static int
 qemuHotplugWaitForTrayEject(virDomainObjPtr vm,
 virDomainDiskDefPtr disk)
@@ -845,8 +919,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
 if (qemuHotplugDiskSourceAttach(priv->mon, diskdata) < 0)
 goto exit_monitor;
 
-if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+if (qemuDomainAttachExtensionDevice(priv->mon, >info) < 0)
+goto exit_monitor;
+
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
+ignore_value(qemuDomainDetachExtensionDevice(priv->mon, >info));
 goto exit_monitor;
+}
 
 if (qemuDomainObjExitMonitor(driver, vm) < 0) {
 ret = -2;
@@ -954,7 +1033,16 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr 
driver,
 goto cleanup;
 
 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorAddDevice(priv->mon, devstr);
+
+if ((ret = qemuDomainAttachExtensionDevice(priv->mon,
+   >info)) < 0) {
+goto exit_monitor;
+}
+
+if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0)
+ignore_value(qemuDomainDetachExtensionDevice(priv->mon, 
>info));
+
+ exit_monitor:
 if (qemuDomainObjExitMonitor(driver, vm) < 0) {
 releaseaddr = false;
 ret = -1;
@@ -1417,6 +1505,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 }
 
 if (qemuDomainIsS390CCW(vm->def) &&
+net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
 virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
 net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
 if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
@@ -1486,7 +1575,15 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 goto try_remove;
 
 qemuDomainObjEnterMonitor(driver, vm);
+
+if (qemuDomainAttachExtensionDevice(priv->mon, >info) < 0) {
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+virDomainAuditNet(vm, NULL, net, "attach", false);
+goto try_remove;
+}
+
 if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) {
+ignore_value(qemuDomainDetachExtensionDevice(priv->mon, >info));
 ignore_value(qemuDomainObjExitMonitor(driver, vm));
 virDomainAuditNet(vm, NULL, net, "attach", false);
 goto try_remove;
@@ -1703,8 

[libvirt] [PATCH v8 03/14] conf: Introduce extension flag and zPCI member for PCI address

2018-11-08 Thread Yi Min Zhao
This patch introduces PCI address extension flag for virDomainDeviceInfo
and virPCIDeviceAddress. The extension flag in virDomainDeviceInfo is
used internally during calculating PCI extension flag. The one in
virPCIDeviceAddress is the duplicate to indicate extension address is
being used. Currently only zPCI extension address is introduced to deal
with 'uid' and 'fid' on the S390 platform.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 src/conf/device_conf.h |   4 +
 src/conf/domain_addr.h |   5 ++
 src/qemu/qemu_domain_address.c | 140 -
 src/util/virpci.h  |   2 +
 4 files changed, 149 insertions(+), 2 deletions(-)

diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 407956bd02..2366e03607 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -166,6 +166,10 @@ struct _virDomainDeviceInfo {
  * assignment, never saved and never reported.
  */
 int pciConnectFlags; /* enum virDomainPCIConnectFlags */
+/* pciAddrExtFlags is only used internally to calculate PCI
+ * address extension flags during address assignment.
+ */
+int pciAddrExtFlags; /* enum virDomainPCIAddressExtensionFlags */
 char *loadparm;
 
 /* PCI devices will only be automatically placed on a PCI bus
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 2a9af9c00b..435b3c5d7f 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -29,6 +29,11 @@
 # define VIR_PCI_ADDRESS_SLOT_LAST 31
 # define VIR_PCI_ADDRESS_FUNCTION_LAST 7
 
+typedef enum {
+VIR_PCI_ADDRESS_EXTENSION_NONE = 0, /* no extension */
+VIR_PCI_ADDRESS_EXTENSION_ZPCI = 1 << 0, /* zPCI support */
+} virPCIDeviceAddressExtensionFlags;
+
 typedef enum {
VIR_PCI_CONNECT_HOTPLUGGABLE = 1 << 0, /* is hotplug needed/supported */
 
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 24dd7c1a58..66f946f23d 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -513,6 +513,64 @@ qemuDomainAssignVirtioMMIOAddresses(virDomainDefPtr def,
 }
 
 
+static bool
+qemuDomainDeviceSupportZPCI(virDomainDeviceDefPtr device)
+{
+switch ((virDomainDeviceType)device->type) {
+case VIR_DOMAIN_DEVICE_CHR:
+return false;
+
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+case VIR_DOMAIN_DEVICE_DISK:
+case VIR_DOMAIN_DEVICE_LEASE:
+case VIR_DOMAIN_DEVICE_FS:
+case VIR_DOMAIN_DEVICE_NET:
+case VIR_DOMAIN_DEVICE_INPUT:
+case VIR_DOMAIN_DEVICE_SOUND:
+case VIR_DOMAIN_DEVICE_VIDEO:
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+case VIR_DOMAIN_DEVICE_WATCHDOG:
+case VIR_DOMAIN_DEVICE_GRAPHICS:
+case VIR_DOMAIN_DEVICE_HUB:
+case VIR_DOMAIN_DEVICE_REDIRDEV:
+case VIR_DOMAIN_DEVICE_SMARTCARD:
+case VIR_DOMAIN_DEVICE_MEMBALLOON:
+case VIR_DOMAIN_DEVICE_NVRAM:
+case VIR_DOMAIN_DEVICE_RNG:
+case VIR_DOMAIN_DEVICE_SHMEM:
+case VIR_DOMAIN_DEVICE_TPM:
+case VIR_DOMAIN_DEVICE_PANIC:
+case VIR_DOMAIN_DEVICE_MEMORY:
+case VIR_DOMAIN_DEVICE_IOMMU:
+case VIR_DOMAIN_DEVICE_VSOCK:
+break;
+
+case VIR_DOMAIN_DEVICE_NONE:
+case VIR_DOMAIN_DEVICE_LAST:
+default:
+virReportEnumRangeError(virDomainDeviceType, device->type);
+return false;
+}
+
+return true;
+}
+
+
+static virPCIDeviceAddressExtensionFlags
+qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCapsPtr qemuCaps,
+  virDomainDeviceDefPtr dev)
+{
+virPCIDeviceAddressExtensionFlags extFlags = 0;
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) &&
+qemuDomainDeviceSupportZPCI(dev)) {
+extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI;
+}
+
+return extFlags;
+}
+
+
 /**
  * qemuDomainDeviceCalculatePCIConnectFlags:
  *
@@ -994,6 +1052,56 @@ qemuDomainFillAllPCIConnectFlags(virDomainDefPtr def,
 }
 
 
+/**
+ * qemuDomainFillDevicePCIExtensionFlagsIter:
+ *
+ * @def: the entire DomainDef
+ * @dev: The device to be checked
+ * @info: virDomainDeviceInfo within the device
+ * @opaque: qemu capabilities
+ *
+ * Sets the pciAddressExtFlags for a single device's info. Has properly
+ * formatted arguments to be called by virDomainDeviceInfoIterate().
+ *
+ * Always returns 0 - there is no failure.
+ */
+static int
+qemuDomainFillDevicePCIExtensionFlagsIter(virDomainDefPtr def ATTRIBUTE_UNUSED,
+  virDomainDeviceDefPtr dev,
+  virDomainDeviceInfoPtr info,
+  void *opaque)
+{
+virQEMUCapsPtr qemuCaps = opaque;
+
+info->pciAddrExtFlags =
+qemuDomainDeviceCalculatePCIAddressExtensionFlags(qemuCaps, dev);
+
+return 0;
+}
+
+
+/**
+ * qemuDomainFillAllPCIExtensionFlags:
+ *
+ * @def: the entire DomainDef
+ * @qemuCaps: as you'd expect
+ *
+ * Set the 

[libvirt] [PATCH v8 02/14] qemu: Introduce zPCI capability

2018-11-08 Thread Yi Min Zhao
Let's introduce zPCI capability.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
Reviewed-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml  | 1 +
 9 files changed, 10 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ca5af3297..eed27f6878 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -509,6 +509,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "vfio-pci.display",
   "blockdev",
   "vfio-ap",
+  "zpci",
 );
 
 
@@ -1094,6 +1095,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "mch", QEMU_CAPS_DEVICE_MCH },
 { "sev-guest", QEMU_CAPS_SEV_GUEST },
 { "vfio-ap", QEMU_CAPS_DEVICE_VFIO_AP },
+{ "zpci", QEMU_CAPS_DEVICE_ZPCI },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6bb9a2c8f0..270c2e3c95 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -493,6 +493,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */
 QEMU_CAPS_BLOCKDEV, /* -blockdev and blockdev-add are supported */
 QEMU_CAPS_DEVICE_VFIO_AP, /* -device vfio-ap */
+QEMU_CAPS_DEVICE_ZPCI, /* -device zpci */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index e000aac384..3c311042f3 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -113,6 +113,7 @@
   
   
   
+  
   201
   0
   306247
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 4eb8a39d94..48db1dbf2d 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -120,6 +120,7 @@
   
   
   
+  
   2011000
   0
   345099
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 79320d5229..4c561f6214 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -128,6 +128,7 @@
   
   
   
+  
   2012000
   0
   374287
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
index b30c31cafc..de87692857 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
@@ -100,6 +100,7 @@
   
   
   
+  
   2007000
   0
   219140
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index b010f731a5..f3a32ad376 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -103,6 +103,7 @@
   
   
   
+  
   2007093
   0
   244554
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index 5a4371ab83..f1e05ab1c4 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -107,6 +107,7 @@
   
   
   
+  
   2009000
   0
   267973
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
index 3b5f9818a5..c841030b2b 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
@@ -130,6 +130,7 @@
   
   
   
+  
   300
   0
   387601
-- 
Yi Min

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

[libvirt] [python] WIP-FYI: mypy annotations for libvirt-python

2018-11-08 Thread Philipp Hahn
Hello,

Maybe you already have heads about mypy , which
"is an experimental optional static type checker for Python that aims to
combine the benefits of dynamic (or "duck") typing and static typing".

I started to write a manual annotation file for the Python binding of
libvirt. I've attached my current version, so others can benefit from
it, too. It is far from complete, but it already helped my to find some
errors in my code.
(My latest version is also available at
)

Long-term it probably would be better to teach the Python binding
"generator.py" to add the type information (PEP 484
) directly into the generated
"libvirt.py" file, but that's for another day.
If someone else is interested in helping with that, please feel free to
get in contact.

Philipp
-- 
Philipp Hahn
Open Source Software Engineer

Univention GmbH
be open.
Mary-Somerville-Str. 1
D-28359 Bremen
Tel.: +49 421 22232-0
Fax : +49 421 22232-99
h...@univention.de

http://www.univention.de/
Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
Steuer-Nr.: 71-597-02876
# Stubs for libvirt (Python 2)
#
# NOTE: This dynamically typed stub was automatically generated by stubgen.

from typing import Any, Callable, Dict, List, Optional

lib_e: Any
cyg_e: Any

class libvirtError(Exception):
err: Any = ...
def __init__(self, defmsg: str, conn: Optional[virConnect] = ..., dom: 
Optional[virDomain] = ..., net: Optional[virNetwork] = ..., pool: 
Optional[virStoragePool] = ..., vol: Optional[virStorageVol] = ...) -> None: ...
def get_error_code(self) -> int: ...
def get_error_domain(self): ...
def get_error_message(self) -> str: ...
def get_error_level(self) -> int: ...
def get_str1(self) -> str: ...
def get_str2(self) -> str: ...
def get_str3(self) -> str: ...
def get_int1(self) -> int: ...
def get_int2(self) -> int: ...

def registerErrorHandler(f: Any, ctx: Any): ...
def openAuth(uri: str, auth: Any, flags: int = ...) -> virConnect: ...
def getVersion(name: Optional[Any] = ...): ...
def virEventAddHandle(fd: Any, events: Any, cb: Callable, opaque: Any): ...
def virEventAddTimeout(timeout: Any, cb: Callable, opaque: Any): ...
def open(name: Optional[str] = ...) -> virConnect: ...
def openReadOnly(name: Optional[str] = ...) -> virConnect: ...
def virEventRegisterDefaultImpl(): ...
def virEventRegisterImpl(addHandle: Any, updateHandle: Any, removeHandle: Any, 
addTimeout: Any, updateTimeout: Any, removeTimeout: Any) -> None: ...
def virEventRemoveHandle(watch: Any): ...
def virEventRemoveTimeout(timer: Any): ...
def virEventRunDefaultImpl(): ...
def virEventUpdateHandle(watch: Any, events: Any) -> None: ...
def virEventUpdateTimeout(timer: Any, timeout: Any) -> None: ...
def virGetLastError() -> libvirtError: ...
def virGetLastErrorMessage(): ...
def virInitialize(): ...
def virResetLastError() -> None: ...

class virDomain:
def __init__(self, conn: virConnect, _obj: Optional[Any] = ...) -> None: ...
def __del__(self) -> None: ...
def connect(self) -> virConnect: ...
def c_pointer(self) -> int: ...
def ID(self) -> int: ...
def OSType(self) -> str: ...
def UUID(self) -> bytes: ...
def UUIDString(self) -> str: ...
def XMLDesc(self, flags: int = ...) -> str: ...
def abortJob(self): ...
def addIOThread(self, iothread_id: Any, flags: int = ...): ...
def attachDevice(self, xml: str): ...
def attachDeviceFlags(self, xml: str, flags: int = ...): ...
def autostart(self): ...
def blkioParameters(self, flags: int = ...): ...
def blockCommit(self, disk: Any, base: Any, top: Any, bandwidth: int = ..., 
flags: int = ...): ...
def blockCopy(self, disk: Any, destxml: str, params: Optional[Any] = ..., 
flags: int = ...): ...
def blockInfo(self, path: Any, flags: int = ...): ...
def blockIoTune(self, disk: Any, flags: int = ...): ...
def blockJobAbort(self, disk: Any, flags: int = ...): ...
def blockJobInfo(self, path: Any, flags: int = ...): ...
def blockJobSetSpeed(self, disk: Any, bandwidth: Any, flags: int = ...): ...
def blockPeek(self, disk: Any, offset: Any, size: Any, flags: int = ...): 
...
def blockPull(self, disk: Any, bandwidth: int = ..., flags: int = ...): ...
def blockRebase(self, disk: Any, base: Any, bandwidth: int = ..., flags: 
int = ...): ...
def blockResize(self, disk: Any, size: Any, flags: int = ...): ...
def blockStats(self, path: Any): ...
def blockStatsFlags(self, path: Any, flags: int = ...): ...
def controlInfo(self, flags: int = ...): ...
def coreDump(self, to: Any, flags: int = ...): ...
def coreDumpWithFormat(self, to: Any, dumpformat: Any, flags: int = ...): 
...
def create(self): ...
def createWithFlags(self, flags: int = ...): ...
def delIOThread(self, iothread_id: Any, flags: int = ...): ...
def 

Re: [libvirt] [PATCH v2] lxc: Clang is complaining about possible NULL pointer.

2018-11-08 Thread Martin Kletzander

On Wed, Nov 07, 2018 at 04:23:47PM -0500, John Ferlan wrote:



On 11/7/18 3:57 PM, Julio Faracco wrote:

The array "mount" inside lxc_container is not being checked before for
loop. Clang syntax scan is complaining about this segmentation fault.

Signed-off-by: Julio Faracco 
---
 src/lxc/lxc_container.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)



Reviewed-by: John Ferlan 
(and pushed)

John

FWIW:
Ironically Coverity never complained about this one even though it's in
the category of things Coverity doesn't like either ;-)



My guess is that coverity was clever enough to know that thing can never happen
(it could happen only if nmounts is non-zero and mounts is NULL).


--
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 0/3] add disk driver metadata_cache_size option

2018-11-08 Thread Nikolay Shirokovskiy



On 07.11.2018 17:42, Peter Krempa wrote:
> On Fri, Nov 02, 2018 at 11:11:50 +0100, Kevin Wolf wrote:
>> Am 01.11.2018 um 12:32 hat Nikolay Shirokovskiy geschrieben:
>>> Hi, all.
>>>
>>> This is a patch series after offlist agreement on introducing
>>> metadata-cache-size option for disks. The options itself is described in 2nd
>>> patch of the series.
>>>
>>> There is a plenty of attempts to add option to set qcow2 metadata cache 
>>> sizes in
>>> past, see [1] [2] to name recent that has comments I tried to address or has
>>> work I reused to some extent.
>>>
>>> I would like to address Peter's comment in [1] that this option is image's
>>> option rather then disk's here. While this makes sense if we set specific 
>>> cache
>>> value that covers disk only partially, here when we have maximum policy that
>>> covers whole disk it makes sense to set same value for all images. The 
>>> setted
>>> value is only upper limit on cache size and thus cache for every image will
>>> grow proportionally to image data size "visible from top" eventually I 
>>> guess.
>>> And these sizes are changing during guest lifetime - thus we need set whole
>>> disk limit for every image for 'maxium' effect.
>>>
>>> On the other hand if we add policies different from 'maximum' it may require
>>> per image option. Well I can't imagine name for element for backing chain 
>>> that
>>> will have cache size attribute better then 'driver'). And driver is already
>>> used for top image so I leave it this way.
>>>
>>>   KNOWN ISSUES
>>>
>>> 1. when using -driver to configure disks in command line cache size does not
>>>get propagated thru backing chain
>>>
>>> 2. when making external disk snapshot cache size sneak into backing file
>>>filename and thus cache size for backing image became remembered there
>>>
>>> 3. blockcommit after such snapshot will not work because libvirt is not 
>>> ready
>>>for backing file name is reported as json sometimes
>>>
>>> Looks like 1 can be addressed in qemu.
>>
>> I don't think we want to add inheritance of driver-specific options.
>> Option inheritance is already a bit messy with options that every driver
>> understands.
>>
>> And -drive is the obsolete interface anyway, when you use -blockdev you
>> specify all nodes explicitly.
> 
> So this would mean we get different behaviour depending on whether
> -blockdev is supported by libvirt and qemu or not.
> 
> This means that there are the following possibilities:
> 
> 1) allow this feature only with -blockdev
> 
> 2) limit this only to the top layer image
> 
> 3) make it configurable per backing chain entry.
> 
> Given our discussion on the KVM forum, I don't think it makes sense to
> do 3, 2 would make it incomplete, so 1 is the only reasonable option if
> qemu will not do the inheritance.
> 
>>> The reason for behaviour in 2 I do not understand honestly.
>>
>> QEMU doesn't recognise that the cache size option doesn't affect which
>> data you're accessing. Max had a series that should probably fix it.
>> Maybe we should finally get it merged.
>>
>> Anyway, I suppose if you use libvirt in -blockdev mode, it doesn't make
>> a difference anyway, because libvirt should then manually call
>> blockdev-create for the overlay and therefore specify the backing file
>> string explicitly.
> 
> Yes, in that case we'll create it manually with the correct backing
> store path. Currently we'd ignore such an entry in the backing store
> path when detecting the chain from the disk so this should not affect
> anything.
> 
>> Can you confirm this in practice?
>>
>>> And 3 will be naturally addessed after blockjobs starts working with
>>> blockdev configuration I guess.
> 
> Did you test this? We do support backing files with 'json:' pseudo
> protocol.

A kind of :)

I can not even create snapshot when using -blockdev
(setting/unsetting metadata_cache_size makes no different)

# cat snap.xml 

  

  


# virsh snapshot-create VM snap.xml --disk-only --no-metadata
error: internal error: unable to execute QEMU command 'transaction': Cannot 
find device=drive-scsi0-0-0-0 nor node_name=

But if I create snapshot with qemu using -drive, then destroy domain,
change qemu binary to the one that supports -blockdev, start domain and 
blockcommit
I get:

# virsh blockcommit VM sda --pivot
error: internal error: unable to find backing name for device drive-scsi0-0-0-0

Yeah, looks like this does not relate to json pseudo protocol in backing file, 
sorry.
It is just due to blockjobs are not yet supported for -blockdev as mentioned in 
[1]

Ahhh, blockcommit failed with message: 

error: internal error: qemu block name 'json:{"driver": "qcow2", 
"l2-cache-size": "9223372036854775807", "file": {"driver": "file", "filename": 
"/somepath/harddisk.hdd"}}' doesn't match expected '/somepath/harddisk.hdd'

for versions of libvirt before blockdev patches (libvirt-3.9.0). Sorry again.


So looks like we can merge the series after addressing your comments
and leave only support