date:20200103

Re: [libvirt] [PATCH v3 0/3] Mark and document restore with managed save as risky

2020-01-03 Thread Daniel Henrique Barboza





On 1/3/20 3:43 PM, Michael Weiser wrote:

This series marks restore of an inactive qemu snapshot while there is
managed saved state as risky due to the reasons explained in patch 1 and
3. Patch 2 is a simple reformatting of the documentation with no other
changes in preparation of addition of more reasons why reverts might
need to be forced.

Changes from v2:
- fix manpage typo exising -> existing
- change manpage qemu hypervisor name case to QEMU

Changes from v1:
- reword error message to "error: revert requires force: snapshot
   without memory state, removal of existing managed saved state strongly
   recommended to avoid corruption"
- add documentation of the new behaviour

Michael Weiser (3):
   qemu: Warn of restore with managed save being risky
   docs: Reformat snapshot-revert force reasons
   docs: Add snapshot-revert qemu managedsave force



All patches:


Reviewed-by: Daniel Henrique Barboza 

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

Re: [libvirt] [PATCH v2] docs: Harmonize hypervisor names for QEMU and LXC

2020-01-03 Thread Daniel Henrique Barboza





On 1/3/20 3:39 PM, Michael Weiser wrote:

Trivially replace usages of qemu and lxc in the virsh manpage with their
more heavily used and (according to Wikipedia) correct upper-case
spellings QEMU and LXC.

Signed-off-by: Michael Weiser 
Suggested-by: Daniel Henrique Barboza 
---


Reviewed-by: Daniel Henrique Barboza 

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

Re: [libvirt] [PULL 0/3] Block patches

2020-01-03 Thread Peter Maydell

On Fri, 20 Dec 2019 at 10:25, Stefan Hajnoczi  wrote:
>
> The following changes since commit aceeaa69d28e6f08a24395d0aa6915b687d0a681:
>
>   Merge remote-tracking branch 
> 'remotes/huth-gitlab/tags/pull-request-2019-12-17' into staging (2019-12-17 
> 15:55:20 +)
>
> are available in the Git repository at:
>
>   https://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 725fe5d10dbd4259b1853b7d253cef83a3c0d22a:
>
>   virtio-blk: fix out-of-bounds access to bitmap in notify_guest_bh 
> (2019-12-19 16:20:25 +)
>
> 
> Pull request
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM


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

[libvirt] [PATCH v3 0/3] Mark and document restore with managed save as risky

2020-01-03 Thread Michael Weiser

This series marks restore of an inactive qemu snapshot while there is
managed saved state as risky due to the reasons explained in patch 1 and
3. Patch 2 is a simple reformatting of the documentation with no other
changes in preparation of addition of more reasons why reverts might
need to be forced.

Changes from v2:
- fix manpage typo exising -> existing
- change manpage qemu hypervisor name case to QEMU

Changes from v1:
- reword error message to "error: revert requires force: snapshot
  without memory state, removal of existing managed saved state strongly
  recommended to avoid corruption"
- add documentation of the new behaviour

Michael Weiser (3):
  qemu: Warn of restore with managed save being risky
  docs: Reformat snapshot-revert force reasons
  docs: Add snapshot-revert qemu managedsave force

 docs/manpages/virsh.rst | 39 ---
 src/qemu/qemu_driver.c  |  9 +
 2 files changed, 33 insertions(+), 15 deletions(-)

-- 
2.24.1


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

[libvirt] [PATCH v3 2/3] docs: Reformat snapshot-revert force reasons

2020-01-03 Thread Michael Weiser

Reformat explanations of the snapshot-revert force reasons in
preparation for more to be added. This is a simple reformat without any
wording changes.

Signed-off-by: Michael Weiser 
Reviewed-by: Daniel Henrique Barboza 
Cc: Cole Robinson 
---
 docs/manpages/virsh.rst | 30 --
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..f5f962cba1 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6952,20 +6952,22 @@ transient domains cannot be inactive, it is required to 
use one of these
 flags when reverting to a disk snapshot of a transient domain.
 
 There are two cases where a snapshot revert involves extra risk, which
-requires the use of *--force* to proceed.  One is the case of a
-snapshot that lacks full domain information for reverting
-configuration (such as snapshots created prior to libvirt 0.9.5);
-since libvirt cannot prove that the current configuration matches what
-was in use at the time of the snapshot, supplying *--force* assures
-libvirt that the snapshot is compatible with the current configuration
-(and if it is not, the domain will likely fail to run).  The other is
-the case of reverting from a running domain to an active state where a
-new hypervisor has to be created rather than reusing the existing
-hypervisor, because it implies drawbacks such as breaking any existing
-VNC or Spice connections; this condition happens with an active
-snapshot that uses a provably incompatible configuration, as well as
-with an inactive snapshot that is combined with the *--start* or
-*--pause* flag.
+requires the use of *--force* to proceed:
+
+  * One is the case of a snapshot that lacks full domain information for
+reverting configuration (such as snapshots created prior to libvirt
+0.9.5); since libvirt cannot prove that the current configuration matches
+what was in use at the time of the snapshot, supplying *--force* assures
+libvirt that the snapshot is compatible with the current configuration
+(and if it is not, the domain will likely fail to run).
+
+  * The other is the case of reverting from a running domain to an active
+state where a new hypervisor has to be created rather than reusing the
+existing hypervisor, because it implies drawbacks such as breaking any
+existing VNC or Spice connections; this condition happens with an active
+snapshot that uses a provably incompatible configuration, as well as with
+an inactive snapshot that is combined with the *--start* or *--pause*
+flag.
 
 
 snapshot-delete
-- 
2.24.1


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

[libvirt] [PATCH v3 3/3] docs: Add snapshot-revert qemu managedsave force

2020-01-03 Thread Michael Weiser

Add documentation for additional reason why snapshot-revert might need
to be forced. This explains why restoring an inactive snapshot while
there is managed saved state is refused by default.

Signed-off-by: Michael Weiser 
Reviewed-by: Daniel Henrique Barboza 
---
 docs/manpages/virsh.rst | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index f5f962cba1..cba590935b 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6951,7 +6951,7 @@ no vm state leaves the domain in an inactive state.  
Passing either the
 transient domains cannot be inactive, it is required to use one of these
 flags when reverting to a disk snapshot of a transient domain.
 
-There are two cases where a snapshot revert involves extra risk, which
+There are a number of cases where a snapshot revert involves extra risk, which
 requires the use of *--force* to proceed:
 
   * One is the case of a snapshot that lacks full domain information for
@@ -6961,7 +6961,7 @@ requires the use of *--force* to proceed:
 libvirt that the snapshot is compatible with the current configuration
 (and if it is not, the domain will likely fail to run).
 
-  * The other is the case of reverting from a running domain to an active
+  * Another is the case of reverting from a running domain to an active
 state where a new hypervisor has to be created rather than reusing the
 existing hypervisor, because it implies drawbacks such as breaking any
 existing VNC or Spice connections; this condition happens with an active
@@ -6969,6 +6969,13 @@ requires the use of *--force* to proceed:
 an inactive snapshot that is combined with the *--start* or *--pause*
 flag.
 
+  * Also, libvirt will refuse to restore snapshots of inactive QEMU domains
+while there is managed saved state. This is because those snapshots do not
+contain memory state and will therefore not replace the existing memory
+state. This ends up switching a disk underneath a running system and will
+likely cause extensive filesystem corruption or crashes due to swap content
+mismatches when run.
+
 
 snapshot-delete
 ---
-- 
2.24.1


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

[libvirt] [PATCH v3 1/3] qemu: Warn of restore with managed save being risky

2020-01-03 Thread Michael Weiser

Internal snapshots of a non-running domain do not carry any memory state
and restoring such a snapshot will not replace existing saved memory
state. This allows a scenario, where a user first suspends a domain into
managedsave, restores a non-running snapshot and then resumes the domain
from managedsave. After that, the guest system will run with its
previous memory state atop a different disk state. The most obvious
possible fallout from this is extensive file system corruption. Swap
content and RAID bitmaps might also be off.

This has been discussed[1] and fixed[2] from the end-user perspective for
virt-manager.

This patch marks the restore operation as risky at the libvirt level,
requiring the user to remove the saved memory state first or force the
operation.

[1] https://www.redhat.com/archives/virt-tools-list/2019-November/msg00011.html
[2] https://www.redhat.com/archives/virt-tools-list/2019-December/msg00049.html

Signed-off-by: Michael Weiser 
Reviewed-by: Daniel Henrique Barboza 
Cc: Cole Robinson 
---
 src/qemu/qemu_driver.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec8faf384c..691a9b45c7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16652,6 +16652,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
_("must respawn qemu to start inactive snapshot"));
 goto endjob;
 }
+if (vm->hasManagedSave &&
+!(snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
+  snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) {
+virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
+   _("snapshot without memory state, removal of "
+ "existing managed saved state strongly "
+ "recommended to avoid corruption"));
+goto endjob;
+}
 }
 
 if (snap->def->dom) {
-- 
2.24.1


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

[libvirt] [PATCH v2] docs: Harmonize hypervisor names for QEMU and LXC

2020-01-03 Thread Michael Weiser

Trivially replace usages of qemu and lxc in the virsh manpage with their
more heavily used and (according to Wikipedia) correct upper-case
spellings QEMU and LXC.

Signed-off-by: Michael Weiser 
Suggested-by: Daniel Henrique Barboza 
---
 docs/manpages/virsh.rst | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..26f08b7701 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -577,7 +577,7 @@ from the domain's XML  element and  subelement 
or one from a
 list of machines from the ``virsh capabilities`` output for a specific
 architecture and domain type.
 
-For the qemu hypervisor, a *virttype* of either 'qemu' or 'kvm' must be
+For the QEMU hypervisor, a *virttype* of either 'qemu' or 'kvm' must be
 supplied along with either the *emulatorbin* or *arch* in order to
 generate output for the default *machine*.  Supplying a *machine*
 value will generate output for the specific machine.
@@ -1072,7 +1072,7 @@ read I/O operations limit.
 write I/O operations limit.
 *--size-iops-sec* specifies size I/O operations limit per second.
 *--group-name* specifies group name to share I/O quota between multiple drives.
-For a qemu domain, if no name is provided, then the default is to have a single
+For a QEMU domain, if no name is provided, then the default is to have a single
 group for each *device*.
 
 Older versions of virsh only accepted these options with underscore
@@ -1084,7 +1084,7 @@ An explicit 0 also clears any limit.  A non-zero value 
for a given total
 cannot be mixed with non-zero values for read or write.
 
 It is up to the hypervisor to determine how to handle the length values.
-For the qemu hypervisor, if an I/O limit value or maximum value is set,
+For the QEMU hypervisor, if an I/O limit value or maximum value is set,
 then the default value of 1 second will be displayed. Supplying a 0 will
 reset the value back to the default.
 
@@ -1211,7 +1211,7 @@ to a unique target name () or source 
file () for one of the disk devices attached to *domain* (see
 also ``domblklist`` for listing these names).
 *bandwidth* specifies copying bandwidth limit in MiB/s, although for
-qemu, it may be non-zero only for an online domain. For further information
+QEMU, it may be non-zero only for an online domain. For further information
 on the *bandwidth* argument see the corresponding section for the ``blockjob``
 command.
 
@@ -1642,7 +1642,7 @@ domblkstat
 Get device block stats for a running domain.  A *block-device* corresponds
 to a unique target name () or source file () for one of the disk devices attached to *domain* (see
-also ``domblklist`` for listing these names). On a lxc or qemu domain,
+also ``domblklist`` for listing these names). On a LXC or QEMU domain,
 omitting the *block-device* yields device block stats summarily for the
 entire domain.
 
@@ -3247,7 +3247,7 @@ destination). Some hypervisors do not support this 
feature and will return an
 error if this parameter is used.
 
 Optional *disks-port* sets the port that hypervisor on destination side should
-bind to for incoming disks traffic. Currently it is supported only by qemu.
+bind to for incoming disks traffic. Currently it is supported only by QEMU.
 
 
 migrate-compcache
@@ -7438,7 +7438,7 @@ qemu-monitor-command
qemu-monitor-command domain { [--hmp] | [--pretty] } command...
 
 Send an arbitrary monitor command *command* to domain *domain* through the
-qemu monitor.  The results of the command will be printed on stdout.  If
+QEMU monitor.  The results of the command will be printed on stdout.  If
 *--hmp* is passed, the command is considered to be a human monitor command
 and libvirt will automatically convert it into QMP if needed.  In that case
 the result will also be converted back from QMP.  If *--pretty* is given,
@@ -7457,7 +7457,7 @@ qemu-agent-command
qemu-agent-command domain [--timeout seconds | --async | --block] command...
 
 Send an arbitrary guest agent command *command* to domain *domain* through
-qemu agent.
+QEMU agent.
 *--timeout*, *--async* and *--block* options are exclusive.
 *--timeout* requires timeout seconds *seconds* and it must be positive.
 When *--aysnc* is given, the command waits for timeout whether success or
-- 
2.24.1


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

Re: [libvirt] [PATCH] docs: Harmonize backend names for QEMU and LXC

2020-01-03 Thread Daniel Henrique Barboza





On 1/3/20 11:09 AM, Michael Weiser wrote:

Trivially replace usages of qemu and lxc in the virsh manpage with their
more heavily used and (according to Wikipedia) correct upper-case
spellings QEMU and LXC.

Signed-off-by: Michael Weiser 
Suggested-by: Daniel Henrique Barboza 
---



Thanks for sending this. You missed a few instances of 'qemu' though:

1213 *bandwidth* specifies copying bandwidth limit in MiB/s, although for
1214 qemu, it may be non-zero only for an online domain. For further information


7440 Send an arbitrary monitor command *command* to domain *domain* through the
7441 qemu monitor.  The results of the command will be printed on stdout.  If


7459 Send an arbitrary guest agent command *command* to domain *domain* through
7460 qemu agent.


These 'qemu' instances in lines 1214, 7441 and 7460 should be upper-cased as 
well.
The remaining lower-case references are fine since they're referencing commands
or URIs.


LXC instances are good to go.


Thanks,


DHB

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

Re: [libvirt] [PATCH] virsh: Add a completer for `domifaddr` --source parameter.

2020-01-03 Thread Julio Faracco

Michal,

This case of comma separated options, can we include a comma after
completion by default?
I.e.:

virsh # domifaddr 1 --source [TAB]
agent  arplease
virsh # domifaddr 1 --source ar[TAB]
virsh # domifaddr 1 --source arp,

This is easy to test and avoid mistakes.

--
Julio Cesar Faracco

Em sex., 3 de jan. de 2020 às 12:45, Michal Prívozník
 escreveu:
>
> On 1/2/20 4:07 PM, Julio Faracco wrote:
> > The command `domifaddr` can use three different sources to grab IP
> > address of a Virtual Machine: lease, agent and arp. This parameter does
> > not have a completer function to return source options.
> >
> > Signed-off-by: Julio Faracco 
> > ---
> >  tools/virsh-completer-domain.c | 17 +
> >  tools/virsh-completer-domain.h |  5 +
> >  tools/virsh-domain-monitor.c   |  1 +
> >  3 files changed, 23 insertions(+)
> >
> > diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
> > index 0311ee50d0..c8709baa38 100644
> > --- a/tools/virsh-completer-domain.c
> > +++ b/tools/virsh-completer-domain.c
> > @@ -296,3 +296,20 @@ virshDomainShutdownModeCompleter(vshControl *ctl,
> >
> >  return virshCommaStringListComplete(mode, modes);
> >  }
> > +
> > +
> > +char **
> > +virshDomainIfAddrSourceCompleter(vshControl *ctl,
> > + const vshCmd *cmd,
> > + unsigned int flags)
> > +{
> > +const char *sources[] = {"lease", "agent", "arp", NULL};
> > +const char *source = NULL;
> > +
> > +virCheckFlags(0, NULL);
> > +
> > +if (vshCommandOptStringQuiet(ctl, cmd, "source", ) < 0)
> > +return NULL;
> > +
> > +return virshCommaStringListComplete(source, sources);
>
> Actually, I don't think this is coorect. This helper completes a string
> list separated by commas, for instance a shutdown mode where more than
> one string (method) can be used:
>
>   virsh shutdown --mode acpi,agent,signal
>
> But this is not the case for domifaddr --source, is it? It accepts
> exactly one string. I know Erik pushed this, so I will post a fix up
> later. Stay tuned.
>
> Michal
>


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

Re: [libvirt] [PATCH] virsh: Add a completer for `domifaddr` --source parameter.

2020-01-03 Thread Michal Prívozník

On 1/2/20 4:07 PM, Julio Faracco wrote:
> The command `domifaddr` can use three different sources to grab IP
> address of a Virtual Machine: lease, agent and arp. This parameter does
> not have a completer function to return source options.
> 
> Signed-off-by: Julio Faracco 
> ---
>  tools/virsh-completer-domain.c | 17 +
>  tools/virsh-completer-domain.h |  5 +
>  tools/virsh-domain-monitor.c   |  1 +
>  3 files changed, 23 insertions(+)
> 
> diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
> index 0311ee50d0..c8709baa38 100644
> --- a/tools/virsh-completer-domain.c
> +++ b/tools/virsh-completer-domain.c
> @@ -296,3 +296,20 @@ virshDomainShutdownModeCompleter(vshControl *ctl,
>  
>  return virshCommaStringListComplete(mode, modes);
>  }
> +
> +
> +char **
> +virshDomainIfAddrSourceCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int flags)
> +{
> +const char *sources[] = {"lease", "agent", "arp", NULL};
> +const char *source = NULL;
> +
> +virCheckFlags(0, NULL);
> +
> +if (vshCommandOptStringQuiet(ctl, cmd, "source", ) < 0)
> +return NULL;
> +
> +return virshCommaStringListComplete(source, sources);

Actually, I don't think this is coorect. This helper completes a string
list separated by commas, for instance a shutdown mode where more than
one string (method) can be used:

  virsh shutdown --mode acpi,agent,signal

But this is not the case for domifaddr --source, is it? It accepts
exactly one string. I know Erik pushed this, so I will post a fix up
later. Stay tuned.

Michal

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

Re: [libvirt] [PATCH v2 00/12] Various bhyve driver improvements for FreeBSD

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:23PM +, Ryan Moeller wrote:
> The main changes are:
>  * Add support for standard hooks in bhyve backend:
>- prepare
>- start
>- started
>- stopped
>- release
>  * Some code cleanup & general housekeeping in bhyve backend
>  * Add support for reboot in bhyve backend, both from within the VM and from 
> the host
> 
> Ryan Moeller (12):
>   Add hooks for bhyve backend

I don't know what has happened, but we seem to be missing
this patch from the list

>   Fix build errors on FreeBSD
>   Simplify bhyve driver caps helpers
>   Remove redundant parameter to virBhyveProcessStart()
>   Factor out conn
>   Fix indentation
>   Eliminate rc variable
>   Make bhyveMonitor a virClass
>   Don't bother seeking to the end of a file opened O_APPEND
>   Add reboot support for bhyve backend

And this patch too.

>   Fix bhyvexml2argvtest
>   Add missing virtlogd.init for OpenRC


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

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

Re: [libvirt] [PATCH v2 08/12] Make bhyveMonitor a virClass

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:31PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 
> ---
>  src/bhyve/bhyve_monitor.c | 144 ++
>  1 file changed, 98 insertions(+), 46 deletions(-)

FWIW, we're aiming to replace  virClass with GObject, but since
you've already done the work here I'm not going to reject it
for that reason.



> 
> diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
> index 0e55e19772..566c672ba0 100644
> --- a/src/bhyve/bhyve_monitor.c
> +++ b/src/bhyve/bhyve_monitor.c
> @@ -32,24 +32,82 @@
>  #include "virerror.h"
>  #include "virfile.h"
>  #include "virlog.h"
> +#include "virobject.h"
>  
>  #define VIR_FROM_THIS   VIR_FROM_BHYVE
>  
>  VIR_LOG_INIT("bhyve.bhyve_monitor");
>  
>  struct _bhyveMonitor {
> +virObject parent;
> +
>  int kq;
>  int watch;
>  bhyveConnPtr driver;
> +virDomainObjPtr vm;
>  };
>  
> +static virClassPtr bhyveMonitorClass;
> +
> +static void
> +bhyveMonitorDispose(void *obj)
> +{
> +bhyveMonitorPtr mon = obj;
> +
> +VIR_FORCE_CLOSE(mon->kq);
> +virObjectUnref(mon->vm);
> +}
> +
> +static int
> +bhyveMonitorOnceInit(void)
> +{
> +if (!VIR_CLASS_NEW(bhyveMonitor, virClassForObject()))
> +return -1;
> +
> +return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(bhyveMonitor);
> +
> +static void bhyveMonitorIO(int, int, int, void *);
> +
> +static bool
> +bhyveMonitorRegister(bhyveMonitorPtr mon)
> +{
> +virObjectRef(mon);
> +mon->watch = virEventAddHandle(mon->kq,
> +   VIR_EVENT_HANDLE_READABLE |
> +   VIR_EVENT_HANDLE_ERROR |
> +   VIR_EVENT_HANDLE_HANGUP,
> +   bhyveMonitorIO,
> +   mon,
> +   virObjectFreeCallback);
> +if (mon->watch < 0) {
> +VIR_DEBUG("failed to add event handle for mon %p", mon);
> +virObjectUnref(mon);
> +return false;
> +}
> +return true;
> +}
> +
> +static void
> +bhyveMonitorUnregister(bhyveMonitorPtr mon)
> +{
> +if (mon->watch < 0)
> +return;
> +
> +virEventRemoveHandle(mon->watch);
> +mon->watch = -1;
> +}

These two new methods are really separate refactoring from
the introduction of virClass. So as a future note, we'd generally
prefer if this had been two patches, but I'm fine merging this
one now.

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 12/12] Add missing virtlogd.init for OpenRC

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:35PM +, Ryan Moeller wrote:
> From: Ryan Moeller 
> 
> Signed-off-by: Ryan Moeller 
> ---
>  src/logging/Makefile.inc.am  | 10 ++
>  src/logging/virtlogd.init.in | 14 ++
>  2 files changed, 24 insertions(+)
>  create mode 100644 src/logging/virtlogd.init.in

Reviewed-by: Daniel P. Berrangé 


The same thing is missing for virtlockd  in src/locking if you
fancy sending an equiv patch for that.


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

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

Re: [libvirt] [PATCH v2 11/12] Fix bhyvexml2argvtest

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:34PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 
> ---
>  tests/bhyvexml2argvtest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
> index 3c9c61f024..9e7eb218b8 100644
> --- a/tests/bhyvexml2argvtest.c
> +++ b/tests/bhyvexml2argvtest.c
> @@ -51,11 +51,11 @@ static int testCompareXMLToArgvFiles(const char *xml,
>  
>  conn->privateData = 
>  
> -cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false);
> +cmd = virBhyveProcessBuildBhyveCmd(, vmdef, false);
>  if (vmdef->os.loader)
>  ldcmd = virCommandNew("dummy");
>  else
> -ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "",
> +ldcmd = virBhyveProcessBuildLoadCmd(, vmdef, "",
>  );
>  
>  if ((cmd == NULL) || (ldcmd == NULL)) {

We need tests to be successfully buildable at every step, so that
"git bisect" can wrk reliably. So this needs squashing into patch 2
which first introduced the test build failure


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

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

Re: [libvirt] [PATCH v2 09/12] Don't bother seeking to the end of a file opened O_APPEND

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:32PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 
> ---
>  src/bhyve/bhyve_process.c | 5 -
>  1 file changed, 5 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 06/12] Fix indentation

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:29PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 

BTW, is it intentional that you have capital 'X' here, but not
in your email From headers ?

> ---
>  src/bhyve/bhyve_monitor.c | 4 ++--
>  src/bhyve/bhyve_process.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 07/12] Eliminate rc variable

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:30PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 
> ---
>  src/bhyve/bhyve_monitor.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 05/12] Factor out conn

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:28PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 
> ---
>  src/bhyve/bhyve_driver.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 04/12] Remove redundant parameter to virBhyveProcessStart()

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:27PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 
> ---
>  src/bhyve/bhyve_driver.c  |  6 +++---
>  src/bhyve/bhyve_process.c | 16 +++-
>  src/bhyve/bhyve_process.h |  1 -
>  3 files changed, 10 insertions(+), 13 deletions(-)

Reviewed-by: Daniel P. Berrangé 

> @@ -140,19 +138,19 @@ virBhyveProcessStart(virConnectPtr conn,
>  goto cleanup;
>  }
>  
> -VIR_FREE(privconn->pidfile);
> -if (!(privconn->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR,
> +VIR_FREE(driver->pidfile);
> +if (!(driver->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR,
>vm->def->name))) {

Indent is off here.


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

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

Re: [libvirt] [PATCH v2 03/12] Simplify bhyve driver caps helpers

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:26PM +, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller 
> ---
>  src/bhyve/bhyve_command.c | 40 +++
>  src/bhyve/bhyve_command.h |  4 ++--
>  src/bhyve/bhyve_driver.c  | 21 +---
>  src/bhyve/bhyve_driver.h  |  4 ++--
>  src/bhyve/bhyve_process.c |  8 +++-
>  5 files changed, 35 insertions(+), 42 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 02/12] Fix build errors on FreeBSD

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:46:25PM +, Ryan Moeller wrote:
> From: Ryan Moeller 
> 
> Don't free the file string until after it has been used to print the
> error message.
> 
> Simplify PCI bus parsing to eliminate an unannotated switch fallthrough.
> 
> Signed-off-by: Ryan Moeller 
> ---
>  src/conf/virnetworkobj.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 

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

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

[libvirt] [PATCH] docs: Harmonize backend names for QEMU and LXC

2020-01-03 Thread Michael Weiser

Trivially replace usages of qemu and lxc in the virsh manpage with their
more heavily used and (according to Wikipedia) correct upper-case
spellings QEMU and LXC.

Signed-off-by: Michael Weiser 
Suggested-by: Daniel Henrique Barboza 
---
 docs/manpages/virsh.rst | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..7e26676570 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -577,7 +577,7 @@ from the domain's XML  element and  subelement 
or one from a
 list of machines from the ``virsh capabilities`` output for a specific
 architecture and domain type.
 
-For the qemu hypervisor, a *virttype* of either 'qemu' or 'kvm' must be
+For the QEMU hypervisor, a *virttype* of either 'qemu' or 'kvm' must be
 supplied along with either the *emulatorbin* or *arch* in order to
 generate output for the default *machine*.  Supplying a *machine*
 value will generate output for the specific machine.
@@ -1072,7 +1072,7 @@ read I/O operations limit.
 write I/O operations limit.
 *--size-iops-sec* specifies size I/O operations limit per second.
 *--group-name* specifies group name to share I/O quota between multiple drives.
-For a qemu domain, if no name is provided, then the default is to have a single
+For a QEMU domain, if no name is provided, then the default is to have a single
 group for each *device*.
 
 Older versions of virsh only accepted these options with underscore
@@ -1084,7 +1084,7 @@ An explicit 0 also clears any limit.  A non-zero value 
for a given total
 cannot be mixed with non-zero values for read or write.
 
 It is up to the hypervisor to determine how to handle the length values.
-For the qemu hypervisor, if an I/O limit value or maximum value is set,
+For the QEMU hypervisor, if an I/O limit value or maximum value is set,
 then the default value of 1 second will be displayed. Supplying a 0 will
 reset the value back to the default.
 
@@ -1642,7 +1642,7 @@ domblkstat
 Get device block stats for a running domain.  A *block-device* corresponds
 to a unique target name () or source file () for one of the disk devices attached to *domain* (see
-also ``domblklist`` for listing these names). On a lxc or qemu domain,
+also ``domblklist`` for listing these names). On a LXC or QEMU domain,
 omitting the *block-device* yields device block stats summarily for the
 entire domain.
 
@@ -3247,7 +3247,7 @@ destination). Some hypervisors do not support this 
feature and will return an
 error if this parameter is used.
 
 Optional *disks-port* sets the port that hypervisor on destination side should
-bind to for incoming disks traffic. Currently it is supported only by qemu.
+bind to for incoming disks traffic. Currently it is supported only by QEMU.
 
 
 migrate-compcache
-- 
2.24.1


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

Re: [libvirt] [PATCH v2 3/3] docs: Add snapshot-revert qemu managedsave force

2020-01-03 Thread Daniel Henrique Barboza





On 1/2/20 1:00 PM, Michael Weiser wrote:

Add documentation for additional reason why snapshot-revert might need
to be forced. This explains why restoring an inactive snapshot while
there is managed saved state is refused by default.

Signed-off-by: Michael Weiser 
Cc: Cole Robinson 
---
  docs/manpages/virsh.rst | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index f5f962cba1..09be872fdf 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6951,7 +6951,7 @@ no vm state leaves the domain in an inactive state.  
Passing either the
  transient domains cannot be inactive, it is required to use one of these
  flags when reverting to a disk snapshot of a transient domain.
  
-There are two cases where a snapshot revert involves extra risk, which

+There are a number of cases where a snapshot revert involves extra risk, which
  requires the use of *--force* to proceed:
  
* One is the case of a snapshot that lacks full domain information for

@@ -6961,7 +6961,7 @@ requires the use of *--force* to proceed:
  libvirt that the snapshot is compatible with the current configuration
  (and if it is not, the domain will likely fail to run).
  
-  * The other is the case of reverting from a running domain to an active

+  * Another is the case of reverting from a running domain to an active
  state where a new hypervisor has to be created rather than reusing the
  existing hypervisor, because it implies drawbacks such as breaking any
  existing VNC or Spice connections; this condition happens with an active
@@ -6969,6 +6969,13 @@ requires the use of *--force* to proceed:
  an inactive snapshot that is combined with the *--start* or *--pause*
  flag.
  
+  * Also, libvirt will refuse to restore snapshots of inactive qemu domains



Side note: there is mixed usage of both 'qemu' and 'QEMU' in this documentation,
with more instances of 'QEMU'.



+while there is managed saved state. This is because those snapshots do not
+contain memory state and will therefore not replace the exising memory


s/exising/existing


With this typo fixed:


Reviewed-by: Daniel Henrique Barboza 




+state. This ends up switching a disk underneath a running system and will
+likely cause extensive filesystem corruption or crashes due to swap content
+mismatches when run.
+
  
  snapshot-delete

  ---



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

Re: [libvirt] [PATCH v2 2/3] docs: Reformat snapshot-revert force reasons

2020-01-03 Thread Daniel Henrique Barboza





On 1/2/20 1:00 PM, Michael Weiser wrote:

Reformat explanations of the snapshot-revert force reasons in
preparation for more to be added. This is a simple reformat without any
wording changes.

Signed-off-by: Michael Weiser 
Cc: Cole Robinson 
---



Reviewed-by: Daniel Henrique Barboza 



  docs/manpages/virsh.rst | 30 --
  1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..f5f962cba1 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6952,20 +6952,22 @@ transient domains cannot be inactive, it is required to 
use one of these
  flags when reverting to a disk snapshot of a transient domain.
  
  There are two cases where a snapshot revert involves extra risk, which

-requires the use of *--force* to proceed.  One is the case of a
-snapshot that lacks full domain information for reverting
-configuration (such as snapshots created prior to libvirt 0.9.5);
-since libvirt cannot prove that the current configuration matches what
-was in use at the time of the snapshot, supplying *--force* assures
-libvirt that the snapshot is compatible with the current configuration
-(and if it is not, the domain will likely fail to run).  The other is
-the case of reverting from a running domain to an active state where a
-new hypervisor has to be created rather than reusing the existing
-hypervisor, because it implies drawbacks such as breaking any existing
-VNC or Spice connections; this condition happens with an active
-snapshot that uses a provably incompatible configuration, as well as
-with an inactive snapshot that is combined with the *--start* or
-*--pause* flag.
+requires the use of *--force* to proceed:
+
+  * One is the case of a snapshot that lacks full domain information for
+reverting configuration (such as snapshots created prior to libvirt
+0.9.5); since libvirt cannot prove that the current configuration matches
+what was in use at the time of the snapshot, supplying *--force* assures
+libvirt that the snapshot is compatible with the current configuration
+(and if it is not, the domain will likely fail to run).
+
+  * The other is the case of reverting from a running domain to an active
+state where a new hypervisor has to be created rather than reusing the
+existing hypervisor, because it implies drawbacks such as breaking any
+existing VNC or Spice connections; this condition happens with an active
+snapshot that uses a provably incompatible configuration, as well as with
+an inactive snapshot that is combined with the *--start* or *--pause*
+flag.
  
  
  snapshot-delete




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

Re: [libvirt] [PATCH] virsh: Add a completer for `domifaddr` --source parameter.

2020-01-03 Thread Erik Skultety

On Thu, Jan 02, 2020 at 12:07:06PM -0300, Julio Faracco wrote:
> The command `domifaddr` can use three different sources to grab IP
> address of a Virtual Machine: lease, agent and arp. This parameter does
> not have a completer function to return source options.
>
> Signed-off-by: Julio Faracco 
> ---
...

> +
> +char **
> +virshDomainIfAddrSourceCompleter(vshControl *ctl,

I'd stay consistent with the existing naming scheme we have in place and expand
'If' to 'Interface' --> virshDomainInterfaceAddrSourceCompleter.

Changed and pushed.
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH] Add overrides for network port UUID getter/lookup methods

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 07:46:40PM +0100, Fabiano Fidêncio wrote:
> [snip]
> 
> > +static PyObject *
> > +libvirt_virNetworkPortLookupByUUID(PyObject *self ATTRIBUTE_UNUSED,
> > +   PyObject *args)
> > +{
> > +virNetworkPortPtr c_retval;
> > +virNetworkPtr net;
> > +PyObject *pyobj_net;
> > +unsigned char *uuid;
> > +int len;
> > +
> > +if (!PyArg_ParseTuple(args, (char *)"Oz#:virNetworkPortLookupByUUID",
> > +  _net, , ))
> > +return NULL;
> > +net = (virNetworkPtr) PyvirNetwork_Get(pyobj_net);
> > +
> 
> Shouldn't we also check whether net is NULL here?

We don't because this C code is only called from our Python generated
stub which will always pass a non-NULL pointer. In any case.

> 
> > +if ((uuid == NULL) || (len != VIR_UUID_BUFLEN))
> > +return VIR_PY_NONE;
> > +
> > +LIBVIRT_BEGIN_ALLOW_THREADS;
> > +c_retval = virNetworkPortLookupByUUID(net, uuid);

...this method will report an error if "net" is NULL.

> > +LIBVIRT_END_ALLOW_THREADS;
> > +
> > +return libvirt_virNetworkPortPtrWrap((virNetworkPortPtr) c_retval);
> > +}
> > +
> 
> [snip]
> 
> With that fixed, Reviewed-by: Fabiano Fidêncio 
> 

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

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

Re: [libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:40:22PM +0100, Fabiano Fidêncio wrote:
> Daniel,
> 
> On Thu, Jan 2, 2020 at 3:59 PM Daniel P. Berrangé  wrote:
> >
> > In the last days before the xmas break I took some time to
> > eliminate 14 more gnulib modules, bringing us down to just
> > 50 left to go. They're getting harder to eliminate as we go
> > on, but to give some hints, I've annotated every module in
> > bootstrap.conf with a suggested replacement strategy.
> >
> > Daniel P. Berrangé (23):
> >   build: set min version for CLang to 3.4 / XCode CLang to 5.1
> >   docs: expand macOS platform support coverage
> >   travis: add macOS Xcode 11.3 testing
> >   util: add note about event file descriptors on Windows
> >   src: always pull in glib/gstdio.h header
> >   src: switch to use g_setenv/g_unsetenv
> 
> In the patch above I'd also take the opportunity and use TRUE/FALSE
> instead of 1/0 in g_setenv().
> Feel free to ignore as this is just a minor nitpick;

Yes, it should use TRUE/FALSE to match the API parameter type.

> >   util: add compat wrapper for g_fsync
> >   src: use g_fsync for portability
> >   util: introduce virFileDataSync
> 
> What would be the impact of using g_fsync() on Linuxes as well?

fsync is stronger & so worse for performance

[quote fdatasync(2)]
   fdatasync()  is similar to fsync(), but does not flush modified metadata
   unless that metadata is needed in  order  to  allow  a  subsequent  data
   retrieval  to be correctly handled. 
   ...snip...
   The aim of fdatasync() is to reduce disk activity for applications  that
   do not require all metadata to be synchronized with the disk.
[/quote]

> 
> >   src: use g_lstat() instead of lstat()
> >   src: switch from fnmatch to g_pattern_match_simple
> 
> This one worries me a little bit about possible breakages, mainly
> related to wildcards used in libvirtd.conf.

Yes, there is a risk here, however, the config files are not part of the
API/ABI guarantee and the risk is reasonably low. It'll need a release
note of course.


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

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

Re: [libvirt] [PATCH 19/23] util: replace gethostname() with g_get_hostname()

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:42:39PM +0100, Fabiano Fidêncio wrote:
> [snip]
> 
> > -r = gethostname(virLogHostname, sizeof(virLogHostname));
> > -if (r == -1) {
> > -ignore_value(virStrcpyStatic(virLogHostname, "(unknown)"));
> > -} else {
> > -NUL_TERMINATE(virLogHostname);
> > -}
> > +(void)g_get_host_name();
> 
> Why not use ignore_return() here?

Yes, it should use ignore_value()

> 
> [snip]
> 

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

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

Re: [libvirt] [PATCH 13/23] src: replace last_component() with g_path_get_basename()

2020-01-03 Thread Daniel P . Berrangé

On Thu, Jan 02, 2020 at 05:42:30PM +0100, Fabiano Fidêncio wrote:
> [snip]
> 
> > diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> > index f072afe857..16a527e399 100644
> > --- a/src/rpc/virnetsocket.c
> > +++ b/src/rpc/virnetsocket.c
> > @@ -55,7 +55,6 @@
> >  #include "virprobe.h"
> >  #include "virprocess.h"
> >  #include "virstring.h"
> > -#include "dirname.h"
> >  #include "passfd.h"
> >
> >  #if WITH_SSH2
> > @@ -668,7 +667,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> >  remoteAddr.len = sizeof(remoteAddr.data.un);
> >
> >  if (spawnDaemon) {
> > -const char *binname;
> > +g_autofree char *binname = NULL;
> >
> >  if (spawnDaemon && !binary) {
> >  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > @@ -677,7 +676,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> >  goto cleanup;
> >  }
> >
> > -if (!(binname = last_component(binary)) || binname[0] == '\0') {
> > +if (!(binname = g_path_get_basename(binary)) || binname[0] == 
> > '\0') {
> 
> IIUC, this check is no longer valid.
> According to the g_path_get_basename() documentation "If file_name
> ends with a directory separator it gets the component before the last
> slash. If file_name consists only of directory separators (and on
> Windows, possibly a drive letter), a single separator is returned. If
> file_name is empty, it gets "."."
> 
> Knowing that, shouldn't we adapt the check accordingly?

Yes,

-if (!(binname = g_path_get_basename(binary)) || binname[0] == '\0') {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Cannot determine basename for binary '%s'"),
-   binary);
-goto cleanup;
-}
-
+binname = g_path_get_basename(binary);


> 
> [snip]
> 
> > diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> > index cd52a91ffd..c2499c0a20 100644
> > --- a/src/util/virmdev.c
> > +++ b/src/util/virmdev.c
> > @@ -18,7 +18,6 @@
> >
> >  #include 
> >
> > -#include "dirname.h"
> >  #include "virmdev.h"
> >  #include "virlog.h"
> >  #include "virerror.h"
> > @@ -207,6 +206,7 @@ char *
> >  virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
> >  {
> >  g_autofree char *result_path = NULL;
> > +g_autofree char *result_file = NULL;
> >  g_autofree char *iommu_path = NULL;
> >  g_autofree char *dev_path = virMediatedDeviceGetSysfsPath(uuidstr);
> >  char *vfio_path = NULL;
> > @@ -226,7 +226,9 @@ virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
> >  return NULL;
> >  }
> >
> > -vfio_path = g_strdup_printf("/dev/vfio/%s", 
> > last_component(result_path));
> > +result_file = g_path_get_basename(result_path);
> > +
> > +vfio_path = g_strdup_printf("/dev/vfio/%s", result_file);
> 
> Please, while changing it, use g_build_filename() instead of 
> g_strdup_printf().

I'm going to leave this as is, as we have so much file name building
code already that just printfs.

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

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

Re: [libvirt] [PATCH] create a thread to handle MigrationParamResetto avoid deadlock

2020-01-03 Thread Daniel P . Berrangé

On Fri, Dec 27, 2019 at 01:59:51PM +0800, wang.y...@zte.com.cn wrote:
> Hi Daniel,
> 
> Thanks a lot for your review and reply!
> 
> > On Mon, Dec 23, 2019 at 04:50:00PM +0100, Michal Prívozník wrote:
> > > On 12/23/19 11:12 AM, Daniel P. Berrangé wrote:
> > > > On Mon, Dec 23, 2019 at 03:13:10PM +0800, Yi Wang wrote:
> > > >> From: Li XueLei 
> > > >>
> > > >> Libvirtd no longer receives external requests, after I do the 
> > > >> following.
> > > >>
> 
> .
> 
> > > >
> > > > Libvirt allows multiple connections concurrently and changes made by one
> > > > connection are supposed to apply globally. No per-VM state should be 
> > > > tied
> > > > to a particular connection.
> > >
> > > This is generally very true, except for migration.
> >
> > Hmm, so in that case we do need to make sure this runs from a non-main
> > event thread as this patch does. I'm surprised we've not seen this
> > before though - i'd think it should be a guaranteed deadlock anytime
> > someone tests this scenario.
> >
> > >
> > > >
> > > > IOW, I don't think we need a thread. We should just stop calling
> > > > qemuMigrationSrcCleanup from the connection close callback entirely
> > >
> > > But I agree that something fishy is going on and this doesn't look like
> > > proper solution. Yi, can you please share the backtrace of other threads
> > > too? Why aren't we getting any reply on the monitor?
> >
> > qemuMonitorSend() just puts date onto the TX queue & waits for the
> > main event loop to send it.  We're invoking qemuMonitorSend from
> > the main event loop in this backtrace, hence it'll wait forever
> > for the thread to send it.
> >
> > qemuMonitorSend must never be invoked from the main event thread.
> 
> So do you think how to imporve this patch? Any sugesstion is appreciated :-)

I'm facing a related problem in my patches for providing an "embedded"
QEMU driver - any libvirt API calls that the application makes from
its main event loop will deadlock if they result in a QEMU monitor
command.

We've got another long standing scalability bug too when dealing
with shutdown takes too long, the main event loop is blocked for an
unreasonably long time.

I think the solution to all of these problems is to stop using the main
event loop for the QEMU monitor and agent.

Instead, we should create a new thread for each QEMU VM, and that thread
should run an event loop, handling just the monitor and agent work for
that one VM.

We can do this quite easily now if we use GLib's  GMainContext as the
event loop.

We'll create a GMainContext and store it in qemuDomainObjPrivatePtr.
In qemuProcessLaunch and qemuProcessReconnect we'll need to spawn a
thread running this GMainContext as an event loop.

At the end of qemuProcess we'll need to tell this event loop to quit
and stop the thread.

Then the qemu_agent.c and qemu_monitor.c code need changing to use the
g_source_add_unix_fd () / g_source_remove_unix_fd () APIs instead of
virEventAddHandle/virEventRemoveHandle.

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

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

[libvirt] [PATCH v2 09/12] Don't bother seeking to the end of a file opened O_APPEND

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 src/bhyve/bhyve_process.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 4da2a702e6..75e12c8a8f 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -121,8 +121,6 @@ virBhyveProcessStart(virConnectPtr conn,
 char *devicemap = NULL;
 char *logfile = NULL;
 int logfd = -1;
-off_t pos = -1;
-char ebuf[1024];
 virCommandPtr cmd = NULL;
 virCommandPtr load_cmd = NULL;
 bhyveConnPtr driver = conn->privateData;
@@ -196,9 +194,6 @@ virBhyveProcessStart(virConnectPtr conn,
 
 /* Log generated command line */
 virCommandWriteArgLog(load_cmd, logfd);
-if ((pos = lseek(logfd, 0, SEEK_END)) < 0)
-VIR_WARN("Unable to seek to end of logfile: %s",
- virStrerror(errno, ebuf, sizeof(ebuf)));
 
 VIR_DEBUG("Loading domain '%s'", vm->def->name);
 if (virCommandRun(load_cmd, NULL) < 0)
-- 
2.23.0


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

[libvirt] [PATCH v2 07/12] Eliminate rc variable

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 src/bhyve/bhyve_monitor.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
index b9ad4520d9..0e55e19772 100644
--- a/src/bhyve/bhyve_monitor.c
+++ b/src/bhyve/bhyve_monitor.c
@@ -130,7 +130,6 @@ bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
 {
 bhyveMonitorPtr mon = NULL;
 struct kevent kev;
-int rc;
 
 if (VIR_ALLOC(mon) < 0)
 return NULL;
@@ -145,8 +144,7 @@ bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
 }
 
 EV_SET(, vm->pid, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, mon);
-rc = kevent(mon->kq, , 1, NULL, 0, NULL);
-if (rc < 0) {
+if (kevent(mon->kq, , 1, NULL, 0, NULL) < 0) {
 virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
_("Unable to register process kevent"));
 goto cleanup;
-- 
2.23.0


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

[libvirt] [PATCH v2 12/12] Add missing virtlogd.init for OpenRC

2020-01-03 Thread Ryan Moeller

From: Ryan Moeller 

Signed-off-by: Ryan Moeller 
---
 src/logging/Makefile.inc.am  | 10 ++
 src/logging/virtlogd.init.in | 14 ++
 2 files changed, 24 insertions(+)
 create mode 100644 src/logging/virtlogd.init.in

diff --git a/src/logging/Makefile.inc.am b/src/logging/Makefile.inc.am
index 0f7ffa73b3..8e6ea08f36 100644
--- a/src/logging/Makefile.inc.am
+++ b/src/logging/Makefile.inc.am
@@ -55,6 +55,13 @@ VIRTLOGD_UNIT_FILES_IN = \
 SYSTEMD_UNIT_FILES += $(notdir $(VIRTLOGD_UNIT_FILES_IN:%.in=%))
 SYSTEMD_UNIT_FILES_IN += $(VIRTLOGD_UNIT_FILES_IN)
 
+OPENRC_INIT_FILES += \
+   virtlogd.init \
+   $(NULL)
+OPENRC_INIT_FILES_IN += \
+   virtlogd.init.in \
+   $(NULL)
+
 noinst_LTLIBRARIES += libvirt_driver_log.la
 
 libvirt_la_BUILT_LIBADD += libvirt_driver_log.la
@@ -128,6 +135,9 @@ logging/log_daemon_dispatch_stubs.h: $(LOG_PROTOCOL) \
virLogManagerProtocol VIR_LOG_MANAGER_PROTOCOL \
$(LOG_PROTOCOL) > logging/log_daemon_dispatch_stubs.h
 
+virtlogd.init: logging/virtlogd.init.in $(top_builddir)/config.status
+   $(AM_V_GEN)$(SED) $(COMMON_UNIT_VARS) $< > $@-t && mv $@-t $@
+
 virtlogd.service: logging/virtlogd.service.in $(top_builddir)/config.status
$(AM_V_GEN)sed $(COMMON_UNIT_VARS) $< > $@-t && mv $@-t $@
 
diff --git a/src/logging/virtlogd.init.in b/src/logging/virtlogd.init.in
new file mode 100644
index 00..61e41f7689
--- /dev/null
+++ b/src/logging/virtlogd.init.in
@@ -0,0 +1,14 @@
+#!/sbin/openrc-run
+
+name=virtlogd
+
+command=@sbindir@/virtlogd
+pidfile="@runstatedir@/virtlogd.pid"
+command_args="--daemon --pid-file=${pidfile}"
+PATH="${PATH}:@sbindir@:@bindir@"
+supervisor=supervise-daemon
+
+depend() {
+provide virtlogd
+keyword -shutdown
+}
-- 
2.23.0


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

[libvirt] [PATCH v2 11/12] Fix bhyvexml2argvtest

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 tests/bhyvexml2argvtest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index 3c9c61f024..9e7eb218b8 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -51,11 +51,11 @@ static int testCompareXMLToArgvFiles(const char *xml,
 
 conn->privateData = 
 
-cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false);
+cmd = virBhyveProcessBuildBhyveCmd(, vmdef, false);
 if (vmdef->os.loader)
 ldcmd = virCommandNew("dummy");
 else
-ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "",
+ldcmd = virBhyveProcessBuildLoadCmd(, vmdef, "",
 );
 
 if ((cmd == NULL) || (ldcmd == NULL)) {
-- 
2.23.0


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

[libvirt] [PATCH v2 05/12] Factor out conn

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 src/bhyve/bhyve_driver.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index d591ff63e3..a862921c87 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -947,7 +947,8 @@ bhyveDomainCreateXML(virConnectPtr conn,
 static int
 bhyveDomainDestroyFlags(virDomainPtr dom, unsigned int flags)
 {
-bhyveConnPtr privconn = dom->conn->privateData;
+virConnectPtr conn = dom->conn;
+bhyveConnPtr privconn = conn->privateData;
 virDomainObjPtr vm;
 virObjectEventPtr event = NULL;
 int ret = -1;
@@ -957,7 +958,7 @@ bhyveDomainDestroyFlags(virDomainPtr dom, unsigned int 
flags)
 if (!(vm = bhyveDomObjFromDomain(dom)))
 goto cleanup;
 
-if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
+if (virDomainDestroyFlagsEnsureACL(conn, vm->def) < 0)
 goto cleanup;
 
 if (virDomainObjCheckActive(vm) < 0)
@@ -1061,7 +1062,8 @@ bhyveDomainSetMetadata(virDomainPtr dom,
const char *uri,
unsigned int flags)
 {
-bhyveConnPtr privconn = dom->conn->privateData;
+virConnectPtr conn = dom->conn;
+bhyveConnPtr privconn = conn->privateData;
 virDomainObjPtr vm;
 int ret = -1;
 
@@ -1071,7 +1073,7 @@ bhyveDomainSetMetadata(virDomainPtr dom,
 if (!(vm = bhyveDomObjFromDomain(dom)))
 return -1;
 
-if (virDomainSetMetadataEnsureACL(dom->conn, vm->def, flags) < 0)
+if (virDomainSetMetadataEnsureACL(conn, vm->def, flags) < 0)
 goto cleanup;
 
 ret = virDomainObjSetMetadata(vm, type, metadata, key, uri,
-- 
2.23.0


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

[libvirt] [PATCH v2 06/12] Fix indentation

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 src/bhyve/bhyve_monitor.c | 4 ++--
 src/bhyve/bhyve_process.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
index ad6977e562..b9ad4520d9 100644
--- a/src/bhyve/bhyve_monitor.c
+++ b/src/bhyve/bhyve_monitor.c
@@ -77,8 +77,8 @@ bhyveMonitorIO(int watch, int kq, int events G_GNUC_UNUSED, 
void *opaque)
 if (kev.filter == EVFILT_PROC && (kev.fflags & NOTE_EXIT) != 0) {
 if ((pid_t)kev.ident != vm->pid) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-_("event from unexpected proc %ju!=%ju"),
-(uintmax_t)vm->pid, (uintmax_t)kev.ident);
+   _("event from unexpected proc %ju!=%ju"),
+   (uintmax_t)vm->pid, (uintmax_t)kev.ident);
 return;
 }
 
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 9352baf172..4da2a702e6 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -140,7 +140,7 @@ virBhyveProcessStart(virConnectPtr conn,
 
 VIR_FREE(driver->pidfile);
 if (!(driver->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR,
-  vm->def->name))) {
+vm->def->name))) {
 virReportSystemError(errno,
  "%s", _("Failed to build pidfile path"));
 goto cleanup;
-- 
2.23.0


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

[libvirt] [PATCH v2 08/12] Make bhyveMonitor a virClass

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 src/bhyve/bhyve_monitor.c | 144 ++
 1 file changed, 98 insertions(+), 46 deletions(-)

diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
index 0e55e19772..566c672ba0 100644
--- a/src/bhyve/bhyve_monitor.c
+++ b/src/bhyve/bhyve_monitor.c
@@ -32,24 +32,82 @@
 #include "virerror.h"
 #include "virfile.h"
 #include "virlog.h"
+#include "virobject.h"
 
 #define VIR_FROM_THIS   VIR_FROM_BHYVE
 
 VIR_LOG_INIT("bhyve.bhyve_monitor");
 
 struct _bhyveMonitor {
+virObject parent;
+
 int kq;
 int watch;
 bhyveConnPtr driver;
+virDomainObjPtr vm;
 };
 
+static virClassPtr bhyveMonitorClass;
+
+static void
+bhyveMonitorDispose(void *obj)
+{
+bhyveMonitorPtr mon = obj;
+
+VIR_FORCE_CLOSE(mon->kq);
+virObjectUnref(mon->vm);
+}
+
+static int
+bhyveMonitorOnceInit(void)
+{
+if (!VIR_CLASS_NEW(bhyveMonitor, virClassForObject()))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(bhyveMonitor);
+
+static void bhyveMonitorIO(int, int, int, void *);
+
+static bool
+bhyveMonitorRegister(bhyveMonitorPtr mon)
+{
+virObjectRef(mon);
+mon->watch = virEventAddHandle(mon->kq,
+   VIR_EVENT_HANDLE_READABLE |
+   VIR_EVENT_HANDLE_ERROR |
+   VIR_EVENT_HANDLE_HANGUP,
+   bhyveMonitorIO,
+   mon,
+   virObjectFreeCallback);
+if (mon->watch < 0) {
+VIR_DEBUG("failed to add event handle for mon %p", mon);
+virObjectUnref(mon);
+return false;
+}
+return true;
+}
+
+static void
+bhyveMonitorUnregister(bhyveMonitorPtr mon)
+{
+if (mon->watch < 0)
+return;
+
+virEventRemoveHandle(mon->watch);
+mon->watch = -1;
+}
+
 static void
 bhyveMonitorIO(int watch, int kq, int events G_GNUC_UNUSED, void *opaque)
 {
 const struct timespec zerowait = { 0, 0 };
-virDomainObjPtr vm = opaque;
-bhyveDomainObjPrivatePtr priv = vm->privateData;
-bhyveMonitorPtr mon = priv->mon;
+bhyveMonitorPtr mon = opaque;
+virDomainObjPtr vm = mon->vm;
+bhyveConnPtr driver = mon->driver;
+const char *name;
 struct kevent kev;
 int rc, status;
 
@@ -82,60 +140,49 @@ bhyveMonitorIO(int watch, int kq, int events 
G_GNUC_UNUSED, void *opaque)
 return;
 }
 
+name = vm->def->name;
 status = kev.data;
 if (WIFSIGNALED(status) && WCOREDUMP(status)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Guest %s got signal %d and crashed"),
-   vm->def->name,
-   WTERMSIG(status));
-virBhyveProcessStop(mon->driver, vm,
-VIR_DOMAIN_SHUTOFF_CRASHED);
+   name, WTERMSIG(status));
+virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED);
 } else if (WIFEXITED(status)) {
 if (WEXITSTATUS(status) == 0) {
 /* 0 - reboot */
 /* TODO: Implementing reboot is a little more complicated. */
-VIR_INFO("Guest %s rebooted; destroying domain.",
- vm->def->name);
-virBhyveProcessStop(mon->driver, vm,
-VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+VIR_INFO("Guest %s rebooted; restarting domain.", name);
+virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
 } else if (WEXITSTATUS(status) < 3) {
 /* 1 - shutdown, 2 - halt, 3 - triple fault. others - error */
-VIR_INFO("Guest %s shut itself down; destroying domain.",
- vm->def->name);
-virBhyveProcessStop(mon->driver, vm,
-VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+VIR_INFO("Guest %s shut itself down; destroying domain.", 
name);
+virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
 } else {
 VIR_INFO("Guest %s had an error and exited with status %d; 
destroying domain.",
- vm->def->name, WEXITSTATUS(status));
-virBhyveProcessStop(mon->driver, vm,
-VIR_DOMAIN_SHUTOFF_UNKNOWN);
+ name, WEXITSTATUS(status));
+virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN);
 }
 }
 }
 }
 
-static void
-bhyveMonitorRelease(void *opaque)
-{
-virDomainObjPtr vm = opaque;
-bhyveDomainObjPrivatePtr priv = vm->privateData;
-bhyveMonitorPtr mon = priv->mon;
-
-VIR_FORCE_CLOSE(mon->kq);
-VIR_FREE(mon);
-}
-
-bhyveMonitorPtr
-bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
+static bhyveMonitorPtr
+bhyveMonitorOpenImpl(virDomainObjPtr vm,

[libvirt] [PATCH v2 00/12] Various bhyve driver improvements for FreeBSD

2020-01-03 Thread Ryan Moeller

The main changes are:
 * Add support for standard hooks in bhyve backend:
   - prepare
   - start
   - started
   - stopped
   - release
 * Some code cleanup & general housekeeping in bhyve backend
 * Add support for reboot in bhyve backend, both from within the VM and from 
the host

Ryan Moeller (12):
  Add hooks for bhyve backend
  Fix build errors on FreeBSD
  Simplify bhyve driver caps helpers
  Remove redundant parameter to virBhyveProcessStart()
  Factor out conn
  Fix indentation
  Eliminate rc variable
  Make bhyveMonitor a virClass
  Don't bother seeking to the end of a file opened O_APPEND
  Add reboot support for bhyve backend
  Fix bhyvexml2argvtest
  Add missing virtlogd.init for OpenRC

 src/bhyve/bhyve_command.c|  40 -
 src/bhyve/bhyve_command.h|   4 +-
 src/bhyve/bhyve_driver.c |  67 ++
 src/bhyve/bhyve_driver.h |   4 +-
 src/bhyve/bhyve_monitor.c| 165 +++
 src/bhyve/bhyve_monitor.h|   2 +
 src/bhyve/bhyve_process.c| 106 +++---
 src/bhyve/bhyve_process.h|   4 +-
 src/conf/virnetworkobj.c |   7 +-
 src/logging/Makefile.inc.am  |  10 +++
 src/logging/virtlogd.init.in |  14 +++
 src/util/virhook.c   |  15 
 src/util/virhook.h   |  11 +++
 tests/bhyvexml2argvtest.c|   4 +-
 14 files changed, 319 insertions(+), 134 deletions(-)
 create mode 100644 src/logging/virtlogd.init.in

-- 
2.23.0


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

[libvirt] [PATCH v2 03/12] Simplify bhyve driver caps helpers

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 src/bhyve/bhyve_command.c | 40 +++
 src/bhyve/bhyve_command.h |  4 ++--
 src/bhyve/bhyve_driver.c  | 21 +---
 src/bhyve/bhyve_driver.h  |  4 ++--
 src/bhyve/bhyve_process.c |  8 +++-
 5 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 48336ffa1b..c8424063b7 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -44,7 +44,7 @@
 VIR_LOG_INIT("bhyve.bhyve_command");
 
 static int
-bhyveBuildNetArgStr(virConnectPtr conn,
+bhyveBuildNetArgStr(bhyveConnPtr driver,
 const virDomainDef *def,
 virDomainNetDefPtr net,
 virCommandPtr cmd,
@@ -60,7 +60,7 @@ bhyveBuildNetArgStr(virConnectPtr conn,
 if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) {
 nic_model = g_strdup("virtio-net");
 } else if (net->model == VIR_DOMAIN_NET_MODEL_E1000) {
-if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_NET_E1000) != 0) {
+if ((bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_NET_E1000) != 0) {
 nic_model = g_strdup("e1000");
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -166,7 +166,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, 
virCommandPtr cmd)
 static int
 bhyveBuildAHCIControllerArgStr(const virDomainDef *def,
virDomainControllerDefPtr controller,
-   virConnectPtr conn,
+   bhyveConnPtr driver,
virCommandPtr cmd)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -207,13 +207,13 @@ bhyveBuildAHCIControllerArgStr(const virDomainDef *def,
 
 switch (disk->device) {
 case VIR_DOMAIN_DISK_DEVICE_DISK:
-if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_AHCI32SLOT))
+if ((bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_AHCI32SLOT))
 virBufferAsprintf(, ",hd:%s", disk_source);
 else
 virBufferAsprintf(, "-hd,%s", disk_source);
 break;
 case VIR_DOMAIN_DISK_DEVICE_CDROM:
-if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_AHCI32SLOT))
+if ((bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_AHCI32SLOT))
 virBufferAsprintf(, ",cd:%s", disk_source);
 else
 virBufferAsprintf(, "-cd,%s", disk_source);
@@ -322,7 +322,7 @@ static int
 bhyveBuildGraphicsArgStr(const virDomainDef *def,
  virDomainGraphicsDefPtr graphics,
  virDomainVideoDefPtr video,
- virConnectPtr conn,
+ bhyveConnPtr driver,
  virCommandPtr cmd,
  bool dryRun)
 {
@@ -331,9 +331,7 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
 bool escapeAddr;
 unsigned short port;
 
-bhyveConnPtr driver = conn->privateData;
-
-if (!(bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM) ||
+if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_LPC_BOOTROM) ||
 def->os.bootloader ||
 !def->os.loader) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -342,7 +340,7 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
 return -1;
 }
 
-if (!(bhyveDriverGetCaps(conn) & BHYVE_CAP_FBUF)) {
+if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_FBUF)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Bhyve version does not support framebuffer"));
 return -1;
@@ -433,7 +431,7 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
 }
 
 virCommandPtr
-virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
+virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver,
  virDomainDefPtr def, bool dryRun)
 {
 /*
@@ -461,7 +459,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
 goto error;
 }
 
-if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_CPUTOPOLOGY) != 0) {
+if ((bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_CPUTOPOLOGY) != 0) {
 virCommandAddArgFormat(cmd, 
"cpus=%d,sockets=%d,cores=%d,threads=%d",
nvcpus,
def->cpu->sockets,
@@ -500,7 +498,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
 /* used by default in bhyve */
 break;
 case VIR_DOMAIN_CLOCK_OFFSET_UTC:
-if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_RTC_UTC) != 0) {
+if ((bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_RTC_UTC) != 0) {
 virCommandAddArg(cmd, "-u");
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -534,7 +532,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
 
 if (def->os.bootloader == NULL &&
 def->os.loader) {
-if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) {
+

[libvirt] [PATCH v2 02/12] Fix build errors on FreeBSD

2020-01-03 Thread Ryan Moeller

From: Ryan Moeller 

Don't free the file string until after it has been used to print the
error message.

Simplify PCI bus parsing to eliminate an unannotated switch fallthrough.

Signed-off-by: Ryan Moeller 
---
 src/conf/virnetworkobj.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 5daf4a8cb1..5c45f49be0 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1895,13 +1895,14 @@ virNetworkObjLoadAllPorts(virNetworkObjPtr net,
 file = g_strdup_printf("%s/%s.xml", dir, de->d_name);
 
 portdef = virNetworkPortDefParseFile(file);
-VIR_FREE(file);
-file = NULL;
-
 if (!portdef) {
 VIR_WARN("Cannot parse port %s", file);
+VIR_FREE(file);
+file = NULL;
 continue;
 }
+VIR_FREE(file);
+file = NULL;
 
 virUUIDFormat(portdef->uuid, uuidstr);
 if (virHashAddEntry(net->ports, uuidstr, portdef) < 0)
-- 
2.23.0


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

[libvirt] [PATCH v2 04/12] Remove redundant parameter to virBhyveProcessStart()

2020-01-03 Thread Ryan Moeller

Signed-off-by: Ryan Moeller 
---
 src/bhyve/bhyve_driver.c  |  6 +++---
 src/bhyve/bhyve_process.c | 16 +++-
 src/bhyve/bhyve_process.h |  1 -
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 760619a5a6..d591ff63e3 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -89,7 +89,7 @@ bhyveAutostartDomain(virDomainObjPtr vm, void *opaque)
 virObjectLock(vm);
 if (vm->autostart && !virDomainObjIsActive(vm)) {
 virResetLastError();
-ret = virBhyveProcessStart(data->conn, data->driver, vm,
+ret = virBhyveProcessStart(data->conn, vm,
VIR_DOMAIN_RUNNING_BOOTED, 0);
 if (ret < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -862,7 +862,7 @@ bhyveDomainCreateWithFlags(virDomainPtr dom,
 goto cleanup;
 }
 
-ret = virBhyveProcessStart(dom->conn, privconn, vm,
+ret = virBhyveProcessStart(dom->conn, vm,
VIR_DOMAIN_RUNNING_BOOTED,
start_flags);
 
@@ -921,7 +921,7 @@ bhyveDomainCreateXML(virConnectPtr conn,
 goto cleanup;
 def = NULL;
 
-if (virBhyveProcessStart(conn, privconn, vm,
+if (virBhyveProcessStart(conn, vm,
  VIR_DOMAIN_RUNNING_BOOTED,
  start_flags) < 0) {
 /* If domain is not persistent, remove its data */
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index bddd4e9461..9352baf172 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -113,7 +113,6 @@ bhyveProcessStopHook(virDomainObjPtr vm, virHookBhyveOpType 
op)
 
 int
 virBhyveProcessStart(virConnectPtr conn,
- bhyveConnPtr driver,
  virDomainObjPtr vm,
  virDomainRunningReason reason,
  unsigned int flags)
@@ -126,12 +125,11 @@ virBhyveProcessStart(virConnectPtr conn,
 char ebuf[1024];
 virCommandPtr cmd = NULL;
 virCommandPtr load_cmd = NULL;
-bhyveConnPtr privconn = conn->privateData;
+bhyveConnPtr driver = conn->privateData;
 bhyveDomainObjPrivatePtr priv = vm->privateData;
 int ret = -1, rc;
 
 logfile = g_strdup_printf("%s/%s.log", BHYVE_LOG_DIR, vm->def->name);
-
 if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT,
   S_IRUSR | S_IWUSR)) < 0) {
 virReportSystemError(errno,
@@ -140,19 +138,19 @@ virBhyveProcessStart(virConnectPtr conn,
 goto cleanup;
 }
 
-VIR_FREE(privconn->pidfile);
-if (!(privconn->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR,
+VIR_FREE(driver->pidfile);
+if (!(driver->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR,
   vm->def->name))) {
 virReportSystemError(errno,
  "%s", _("Failed to build pidfile path"));
 goto cleanup;
 }
 
-if (unlink(privconn->pidfile) < 0 &&
+if (unlink(driver->pidfile) < 0 &&
 errno != ENOENT) {
 virReportSystemError(errno,
  _("Cannot remove state PID file %s"),
- privconn->pidfile);
+ driver->pidfile);
 goto cleanup;
 }
 
@@ -170,7 +168,7 @@ virBhyveProcessStart(virConnectPtr conn,
 virCommandSetOutputFD(cmd, );
 virCommandSetErrorFD(cmd, );
 virCommandWriteArgLog(cmd, logfd);
-virCommandSetPidFile(cmd, privconn->pidfile);
+virCommandSetPidFile(cmd, driver->pidfile);
 virCommandDaemonize(cmd);
 
 if (vm->def->os.loader == NULL) {
@@ -215,7 +213,7 @@ virBhyveProcessStart(virConnectPtr conn,
 if (virCommandRun(cmd, NULL) < 0)
 goto cleanup;
 
-if (virPidFileReadPath(privconn->pidfile, >pid) < 0) {
+if (virPidFileReadPath(driver->pidfile, >pid) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Domain %s didn't show up"), vm->def->name);
 goto cleanup;
diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h
index 4f62f6be4b..8419e44faa 100644
--- a/src/bhyve/bhyve_process.h
+++ b/src/bhyve/bhyve_process.h
@@ -24,7 +24,6 @@
 #include "bhyve_utils.h"
 
 int virBhyveProcessStart(virConnectPtr conn,
- bhyveConnPtr driver,
  virDomainObjPtr vm,
  virDomainRunningReason reason,
  unsigned int flags);
-- 
2.23.0


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

42 matches

Mail list logo