Re: [libvirt] [PATCH] libvirt-guests: wait for ntp service

2014-09-21 Thread Martin Kletzander

On Sat, Sep 20, 2014 at 10:09:40AM +0200, Michal Privoznik wrote:

On 20.09.2014 01:36, Jim Fehlig wrote:

Martin Kletzander wrote:

On Fri, Sep 19, 2014 at 02:37:12PM -0600, Jim Fehlig wrote:

Michal Privoznik wrote:

On 08.09.2014 18:30, Jim Fehlig wrote:

If an NTP server is configured on the host, it is possible for
libvirt-guests to start before the NTP service, in which case
guest clocks won't be synchronized to the host clock.

Add ntp-wait.service to "After" in libvirt-guests systemd service
file, ensuring NTP has synchronized the host clock before starting
any guests.

Signed-off-by: Jim Fehlig 
---
   tools/libvirt-guests.service.in | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.service.in
b/tools/libvirt-guests.service.in
index d8d7adf..226b3bd 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,6 +1,6 @@
   [Unit]
   Description=Suspend Active Libvirt Guests
-After=network.target libvirtd.service
+After=network.target libvirtd.service ntp-wait.service
   Documentation=man:libvirtd(8)
   Documentation=http://libvirt.org




Well, guest can have their own ntp-client (and in most cases they do,
right?).


I think most do, but know of at least two users who want to use kvmclock
with no ntp in the guests :).



I'm sure there are way more users without ntp clients inside their
guests.  I'm just wondering what's the difference when libvirt-guests
starts before or after ntp has synchronized their clocks.  Is it that
they have the time reset to a little bit inaccurate time?  Or are they
off way too much?


They are off by the ntp adjustment.  As I understand it, the guests
start and read the host clock, which is later adjusted by ntp.




I mean, since guests can be paused, saved & restored back, their time
is often off. So the best is to have an ntp-client running inside the
guest.




Yes, but if it's way off, ntp will refuse to update the time; that's
why we are resetting the time, isn't it?


Yep.  I mentioned this, but seems they don't use save, restore, migrate,
et. al., since it wasn't a concern.  But I'm fine handling this
downstream.  Thanks!



Well, if they use libvirt-guests, they use at least save/restore :)


They have ON_SHUTDOWN=shutdown.



Unfortunately I'm not very familiar with systemd files, but my guess
is that After=ntp-wait.service means it should be started after the
time is synchronized if and only if the ntp-wait.service unit is
enabled, otherwise it doesn't require it.


Yes, this is my understanding too.


And so is mine. The only concern I have is that syncing time on cold
boot of the host may take ages. But on the other hand, it's better to
start domains later and with correct time than start asap with
inaccurate time. ACK then,



Maybe we could use After=ntpdate.service instead of ntp-wait.service;
although starting an ntpdate.service should result in the
ntp-wait.service to be automatically active, but this is just my
guess, I have no idea how /usr/sbin/ntp-wait works.

Wither way, ACK from me too,

Martin


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

Re: [libvirt] [PATCH 1/1] lxc: allow fallback to no apparmor.

2014-09-21 Thread Chen, Hanxiao


> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Serge Hallyn
> Sent: Saturday, September 20, 2014 12:15 AM
> To: libvir-list@redhat.com
> Subject: [libvirt] [PATCH 1/1] lxc: allow fallback to no apparmor.
> 
> The security_driver line in /etc/libvirt/qemu.conf is best-effort - if
> selinux is not available on the host, then 'none' will be used.
> 
> The security_driver line in /etc/libvirt/lxc.conf doesn't behave the
> same way - if apparmor is specified but policies are not available
> on the host, then container creation fails.
> 
> This patch always tries to fall back to 'none' if the requested
> driver is not available.  A better patch would allow an option list
> like qemu.conf allows, but this patch doesn't do that.
> 
> Signed-off-by: Serge Hallyn 
> ---

Reviewed-by: Chen Hanxiao 



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


Re: [libvirt] [PATCH virt-manager] details: Add UI for enabling UEFI via 'customize before install'

2014-09-21 Thread Laszlo Ersek
Hi Cole and Michal,

I'm attaching three patches:

On 09/20/14 02:37, Cole Robinson wrote:
> On 09/17/2014 06:55 PM, Cole Robinson wrote:
>> We expose a simple combobox with two entries: BIOS, and UEFI. The
>> UEFI option is only selectable if 1) libvirt supports the necessary
>> domcapabilities bits, 2) it detects that qemu supports the necessary
>> command line options, and 3) libvirt detects a UEFI binary on the
>> host that maps to a known template via qemu.conf
>>
>> If those conditions aren't met, we disable the UEFI option, and show
>> a small warning icon with an explanatory tooltip.
>>
>> The option can only be changed via New VM->Customize Before Install.
>> For existing x86 VMs, it's a readonly label.
>
> I've pushed this now, with follow up patches to handle a couple points
> we discussed:
>
> - Remove the nvram deletion from delete.py, since it won't work in the
> non-root case, and all uses of nvram should also be with a libvirt +
> driver that provides UNDEFINE_NVRAM
>
> - Before using UNDEFINE_NVRAM, match the same condition that libvirt
> uses: loader_ro is True and loader_type == "pflash" and bool(nvram)

(1) unfortunately virt-manager commit 3feedb76 ("domain: Match
UNDEFINE_NVRAM conditions with libvirt") is not correct (it doesn't
work), and not what I had in mind:

> diff --git a/virtManager/domain.py b/virtManager/domain.py
> index 29f3861..abf3146 100644
> --- a/virtManager/domain.py
> +++ b/virtManager/domain.py
> @@ -1403,7 +1403,9 @@ class vmmDomain(vmmLibvirtObject):
>  flags |= getattr(libvirt,
>   "VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA", 0)
>  flags |= getattr(libvirt, "VIR_DOMAIN_UNDEFINE_MANAGED_SAVE", 0)
> -if self.get_xmlobj().os.nvram:
> +if (self.get_xmlobj().os.loader_ro is True and
> +self.get_xmlobj().os.loader_type == "pflash" and
> +self.get_xmlobj().os.nvram):
>  flags |= getattr(libvirt, "VIR_DOMAIN_UNDEFINE_NVRAM", 0)
>  try:
>  self._backend.undefineFlags(flags)

Before virt-manager commit 3feedb76, the condition on which to set the
flag was:

  self.get_xmlobj().os.nvram

and it didn't work for all possible cases.

After the patch, what you have is

  self.get_xmlobj().os.nvram *and* blah

The commit only constrains (further tightens, further restricts) the
original condition, which had been too restrictive to begin with. Again,
the nvram element (or its text() child) can be completely absent from
the domain XML -- libvirtd will generate the pathname of the varstore
file on the fly, and it need not be part of the serialized XML file. So
in the XML all you'll have is

  
hvm
/usr/share/OVMF/OVMF_CODE.fd

  

or

  
hvm
/custom/OVMF_CODE.fd


  

My suggestion was to *replace* the original condition with the
(loader_ro && loader_type=="pflash") one. If you check my earlier
message, I wrote *iff*, meaning "if and only if".

Cole, can you please apply the first attached patch?


(2) Unfortunately, even libvirtd needs to be modified, in addition.

My patch for (1) *does* pass VIR_DOMAIN_UNDEFINE_NVRAM to libvirtd (I
verified that with gdb), but libvirtd doesn't act upon it correctly.
Namely, in the libvirtd source code, in qemuDomainUndefineFlags(),
commit 273b6581 added:

if (!virDomainObjIsActive(vm) &&
vm->def->os.loader && vm->def->os.loader->nvram &&
virFileExists(vm->def->os.loader->nvram)) {
if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
   _("cannot delete inactive domain with nvram"));
goto cleanup;
}

if (unlink(vm->def->os.loader->nvram) < 0) {
virReportSystemError(errno,
 _("failed to remove nvram: %s"),
 vm->def->os.loader->nvram);
goto cleanup;
}
}

Here "vm->def->os.loader->nvram" evaluates to NULL:

  6468if (!virDomainObjIsActive(vm) &&
  6469vm->def->os.loader && vm->def->os.loader->nvram &&
  6470virFileExists(vm->def->os.loader->nvram)) {
  6471if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
  6472virReportError(VIR_ERR_OPERATION_INVALID, "%s",

  (gdb) print vm->def->os.loader
  $1 = (virDomainLoaderDefPtr) 0x7ff59c00bf20

  (gdb) print vm->def->os.loader->nvram
  $2 = 0x0

I thought that the auto-generation of the nvram pathname (internal to
libvirtd, ie. not visible in the XML file) would occur on the undefine
path as well. Apparently this is not the case.

Indeed, the only call to qemuPrepareNVRAM() is from qemuProcessStart().

Michal, would it be possible generate the *pathname* of the separate
varstore in qemuDomainUndefineFlags() too?

Please see the second attached patch; it works for me. (This patch is
best looked at with "git diff -b" (or "git show -b").)


(3) I just realized th

[libvirt] [PATCH v3 2/2] Add configure option --enable-pc-1-0-qemu-kvm

2014-09-21 Thread Alex Bligh
Add a configure option --enable-pc-1-0-qemu-kvm and the
corresponding --disable-pc-1-0-qemu-kvm, defaulting
to disabled.

Rename machine type pc-1.0 to pc-1.0-qemu-git.

Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm
or pc-1.0-qemu-git depending on the value of the config
option.

Signed-off-by: Alex Bligh 
---
 configure |   12 
 hw/i386/pc_piix.c |8 +++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index f7685b5..b143302 100755
--- a/configure
+++ b/configure
@@ -335,6 +335,7 @@ libssh2=""
 vhdx=""
 quorum=""
 numa=""
+pc_1_0_qemu_kvm="no"
 
 # parse CC options first
 for opt do
@@ -1125,6 +1126,10 @@ for opt do
   ;;
   --enable-numa) numa="yes"
   ;;
+  --disable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm="no"
+  ;;
+  --enable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm="yes"
+  ;;
   *)
   echo "ERROR: unknown option $opt"
   echo "Try '$0 --help' for more information"
@@ -1394,6 +1399,8 @@ Advanced options (experts only):
   --enable-quorum  enable quorum block filter support
   --disable-numa   disable libnuma support
   --enable-numaenable libnuma support
+  --disable-pc-1-0-qemu-kvm disable pc-1.0 machine type reflecting qemu-kvm
+  --enable-pc-1-0-qemu-kvm enable pc-1.0 machine type reflecting qemu-kvm
 
 NOTE: The object files are built at the place where configure is launched
 EOF
@@ -4262,6 +4269,7 @@ echo "Quorum$quorum"
 echo "lzo support   $lzo"
 echo "snappy support$snappy"
 echo "NUMA host support $numa"
+echo "pc-1.0 qemu-kvm   $pc_1_0_qemu_kvm"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -5241,6 +5249,10 @@ if test "$numa" = "yes"; then
   echo "CONFIG_NUMA=y" >> $config_host_mak
 fi
 
+if test "$pc_1_0_qemu_kvm" = "yes"; then
+  echo "CONFIG_PC_1_0_QEMU_KVM=y" >> $config_host_mak
+fi
+
 # build tree in object directory in case the source is not in the current 
directory
 DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
 DIRS="$DIRS fsdev"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 48a4942..b7a4af0 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -646,7 +646,10 @@ static QEMUMachine pc_machine_v1_1 = {
 
 static QEMUMachine pc_machine_v1_0 = {
 PC_I440FX_1_2_MACHINE_OPTIONS,
-.name = "pc-1.0",
+.name = "pc-1.0-qemu-git",
+#ifndef CONFIG_PC_1_0_QEMU_KVM
+.alias = "pc-1.0",
+#endif
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_1_0,
 { /* end of list */ }
@@ -665,6 +668,9 @@ static QEMUMachine pc_machine_v1_0 = {
 static QEMUMachine pc_machine_v1_0_qemu_kvm = {
 PC_I440FX_1_2_MACHINE_OPTIONS,
 .name = "pc-1.0-qemu-kvm",
+#ifdef CONFIG_PC_1_0_QEMU_KVM
+.alias = "pc-1.0",
+#endif
 .init = pc_init_pci_1_2_qemu_kvm,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_1_0_QEMU_KVM,
-- 
1.7.9.5

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


[libvirt] [PATCH v3 1/2] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

2014-09-21 Thread Alex Bligh
Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
with qemu-kvm version 1.0.

Signed-off-by: Alex Bligh 
---
 hw/acpi/piix4.c |   47 +--
 hw/i386/pc_piix.c   |   30 ++
 hw/timer/i8254_common.c |   10 +-
 3 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..e016005 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -37,6 +37,8 @@
 #include "hw/acpi/memory_hotplug.h"
 #include "hw/acpi/acpi_dev_interface.h"
 
+extern int qemu_kvm_1_0_compat;
+
 //#define DEBUG
 
 #ifdef DEBUG
@@ -200,12 +202,24 @@ static const VMStateDescription vmstate_pci_status = {
 }
 };
 
+static const VMStateDescription vmstate_acpi_compat;
+
 static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
 {
 PIIX4PMState *s = opaque;
 int ret, i;
 uint16_t temp;
 
+/* If we are expecting the inbound migration to come from
+ * qemu-kvm 1.0, it will have a version_id of 2 but really
+ * be version 3, so call back the original vmstate_load_state
+ * with a different more tolerant vmstate descriptor
+ */
+if ((version_id == 2) && (qemu_kvm_1_0_compat)) {
+return vmstate_load_state(f, &vmstate_acpi_compat,
+  opaque, version_id);
+}
+
 ret = pci_device_load(PCI_DEVICE(s), f);
 if (ret < 0) {
 return ret;
@@ -267,8 +281,9 @@ static const VMStateDescription vmstate_memhp_state = {
 };
 
 /* qemu-kvm 1.2 uses version 3 but advertised as 2
- * To support incoming qemu-kvm 1.2 migration, change version_id
- * and minimum_version_id to 2 below (which breaks migration from
+ * To support incoming qemu-kvm 1.2 migration, we support
+ * via a command line option a change to minimum_version_id
+ * of 2 in a _compat structure (which breaks migration from
  * qemu 1.2).
  *
  */
@@ -307,6 +322,34 @@ static const VMStateDescription vmstate_acpi = {
 }
 };
 
+static const VMStateDescription vmstate_acpi_compat = {
+.name = "piix4_pm",
+.version_id = 3,
+.minimum_version_id = 2,
+.minimum_version_id_old = 1,
+.load_state_old = NULL, /* to avoid recursion */
+.post_load = vmstate_acpi_post_load,
+.fields  = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(parent_obj, PIIX4PMState),
+VMSTATE_UINT16(ar.pm1.evt.sts, PIIX4PMState),
+VMSTATE_UINT16(ar.pm1.evt.en, PIIX4PMState),
+VMSTATE_UINT16(ar.pm1.cnt.cnt, PIIX4PMState),
+VMSTATE_STRUCT(apm, PIIX4PMState, 0, vmstate_apm, APMState),
+VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState),
+VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState),
+VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
+VMSTATE_STRUCT_TEST(
+acpi_pci_hotplug.acpi_pcihp_pci_status[ACPI_PCIHP_BSEL_DEFAULT],
+PIIX4PMState,
+vmstate_test_no_use_acpi_pci_hotplug,
+2, vmstate_pci_status,
+struct AcpiPciHpPciStatus),
+VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState,
+vmstate_test_use_acpi_pci_hotplug),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static void piix4_reset(void *opaque)
 {
 PIIX4PMState *s = opaque;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7081c08..48a4942 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -70,6 +70,8 @@ static bool smbios_legacy_mode;
 static bool gigabyte_align = true;
 static bool has_reserved_memory = true;
 
+int qemu_kvm_1_0_compat;
+
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
  int pci_enabled,
@@ -386,6 +388,14 @@ static void pc_init_pci_1_2(MachineState *machine)
 pc_init_pci(machine);
 }
 
+/* PC machine init function for qemu-kvm 1.0 */
+static void pc_init_pci_1_2_qemu_kvm(MachineState *machine)
+{
+qemu_kvm_1_0_compat = 1;
+pc_compat_1_2(machine);
+pc_init_pci(machine);
+}
+
 /* PC init function for pc-0.10 to pc-0.13 */
 static void pc_init_pci_no_kvmclock(MachineState *machine)
 {
@@ -644,6 +654,25 @@ static QEMUMachine pc_machine_v1_0 = {
 .hw_version = "1.0",
 };
 
+#define PC_COMPAT_1_0_QEMU_KVM \
+PC_COMPAT_1_0,\
+{\
+.driver   = "cirrus-vga",\
+.property = "vgamem_mb",\
+.value= stringify(16),\
+}
+
+static QEMUMachine pc_machine_v1_0_qemu_kvm = {
+PC_I440FX_1_2_MACHINE_OPTIONS,
+.name = "pc-1.0-qemu-kvm",
+.init = pc_init_pci_1_2_qemu_kvm,
+.compat_props = (GlobalProperty[]) {
+PC_COMPAT_1_0_QEMU_KVM,
+{ /* end of list */ }
+},
+.hw_version = "1.0",
+};
+
 #define PC_COMPAT_0_15 \
 PC_COMPAT_1_0
 
@@ -886,6 +915,7 @@ static void pc_machine_init(void)
 qemu_register_pc_machine(&pc_machine_v1_2);
 qemu_register_pc_machine(&pc_machine_v1_1);
 qemu_register_pc_machine(&pc_machine_v1_0);
+qemu_register_pc_ma

[libvirt] [PATCH v3 0/2] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

2014-09-21 Thread Alex Bligh
This patch series adds inbound migrate capability from qemu-kvm version
1.0. The main ideas are those set out in Cole Robinson's patch here:
http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qemu-kvm.patch?h=f20
however, rather than patching statically (and breaking inbound
migration on existing machine types), I have added a new machine
type (pc-1.0-qemu-kvm) without affecting any other machine types.
The existing pc-1.0 machine type is renamed to pc-1.0-qemu-git,
with pc-1.0 becoming an alias for one or another, as selected
by a configure option (defaulting to pc-1.0-qemu-git, IE no
change).

Two aproaches are taken:

* In hw/timer/i8254_common.c, the VMSTATE_UINT32_TEST macro
  is used to test the version for the irq_disable flags,
  allowing version 3 or more, or version 2 for an inbound
  migrate from qemu-kvm (only).

* In hw/acpi/piix4.c, qemu-kvm incorrectly uses version 2 for
  a version 3 structure, causing acpi_load_old to be used.
  acpi_load_old detects this situation based on the machine type
  and restarts the attempt to load the vmstate using a
  customised VMStateDescription. The above cleaner approach is
  unavailable here.

I developed this on qemu 2.0 but have forward ported it (trivially)
to master. My testing has been on a VM live-migrated-to-file from
Ubuntu Precise qemu-kvm 1.0.

I have given this a moderate degree of testing but it could do
with more.

Note that certain hardware devices (including QXL) will not
migrate properly due to a fundamental difference in their internal
state between versions.

Also note that (as expected) migration from qemu-2.x to qemu-1.0
will not work, even if the machine types are the same.

Alex Bligh (2):
  Add machine type pc-1.0-qemu-kvm for live migrate compatibility with
qemu-kvm
  Add configure option --enable-pc-1-0-qemu-kvm

 configure   |   12 
 hw/acpi/piix4.c |   47 +--
 hw/i386/pc_piix.c   |   38 +-
 hw/timer/i8254_common.c |   10 +-
 4 files changed, 103 insertions(+), 4 deletions(-)

-- 
1.7.9.5

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