date:20140924


On 09/23/2014 11:50 PM, Eric Blake wrote:

On 09/23/2014 03:26 PM, Pavel Hrdina wrote:


+
+VIR_DEBUG(Relaying domain tunable event %s %d, callback %d,
+  dom-name, dom-id, callback-callbackID);
+


Might also be nice to log %p %n, params, nparams


Yes, that would be probably nice, but since I've pushed this patch
already I can create a following patch with this small update?



Yes, a followup is fine.





+++ b/src/remote/remote_protocol.x
@@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
   /* Upper limit on count of parameters returned via bulk stats API */
   const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;

+/* Upper limit of message size for tunable event. */
+const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;


That feels excessive...


+
   /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
   typedef opaque remote_uuid[VIR_UUID_BUFLEN];

@@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg {
   int status;
   };

+struct remote_domain_event_callback_tunable_msg {
+int callbackID;
+remote_nonnull_domain dom;
+remote_typed_param paramsREMOTE_DOMAIN_EVENT_TUNABLE_MAX;


...each param in the array will occupy multiple bytes.  I think that
something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still
plenty (we don't have that many tunables yet); even if each tunable
requires 64 bytes to transmit (mostly in the name of the parameter, but
also in the type and value), that's still well under a megabyte limit of
information passed on an instance of the event.



Well, yes and no :). Let's say, that we will add in the future (and I'm
planning to do it) blkiotune where you can update at the same time all
of the tunables for all host's disks where all params for now will be
only VIR_TYPED_PARAM_STRING and that could consume a lot of memory. I
know that it will probably never be that much, but I wanted to be sure
that we will have enough space for all possible tunable events.


Still, are you going to return 8 million separate strings?  Or just 8
million bytes but still contained within 2000 strings?  Seriously, I
think 2048 is a perfectly LARGE limit - there are not THAT many tunables
per domain.  The paramsLIMIT is not the overall size of the command,
but the number of parameters (each of which can be quite large if they
are type string)



Sigh, I should not work that late, because I've misunderstood the
meaning of the LIMIT. I'll post a new value with patch for the
debug message.

Thanks, Pavel

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

[libvirt] [PATCH] tunable_event: extend debug message and tweak limit for remote message

It would be nice to also print a params pointer and number of params in
the debug message and the previous limit for number of params in the rpc
message was too large. The 2048 params will be enough for future events.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 daemon/remote.c  | 4 ++--
 src/remote/remote_protocol.x | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index ddd510c..9884bae 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -990,8 +990,8 @@ remoteRelayDomainEventTunable(virConnectPtr conn,
 !remoteRelayDomainEventCheckACL(callback-client, conn, dom))
 return -1;
 
-VIR_DEBUG(Relaying domain tunable event %s %d, callback %d,
-  dom-name, dom-id, callback-callbackID);
+VIR_DEBUG(Relaying domain tunable event %s %d, callback %d, params %p %n,
+  dom-name, dom-id, callback-callbackID, params, nparams);
 
 /* build return data */
 memset(data, 0, sizeof(data));
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 0c6a91e..cd190ef 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -248,7 +248,7 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
 const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;
 
 /* Upper limit of message size for tunable event. */
-const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;
+const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 2048;
 
 /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
 typedef opaque remote_uuid[VIR_UUID_BUFLEN];
-- 
1.8.5.5

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

Re: [libvirt] [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm

2014-09-24 Thread Markus Armbruster

Alex Bligh a...@alex.org.uk writes:

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
parameter (qemu-kvm-migration) which when turned on affects the pc-1.0
machine type. Usage:
-machine pc-1.0,qemu-kvm-migration=on

Forgive me if this has been discussed already: why not simply a separate
machine type pc-1.0-qemu-kvm?

Three aproaches are taken:

* cirrus-vga.vgamem_mb defaults to 16 rather than 8. In order to
keep -global cirrus-vga.vgamem_mb working even with
qemu-kvm-migration=on, this is monkey-patchedinto the default
valueof the MachineState structure's compat_props
list.

This part fires only for pc-1.0, because it's in
pc_early_init_pci_1_0().

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

These parts apply to all machine types, don't they?

The above monkey-patching must be done between the selection of
the MachineClass and the processing of the machine parameters
(on theone hand) and the processing of the compat_props list
and theglobals on the command line. To do this I
have added
an earlyinit function to MachineState and QEMUMachine.

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.

Changes since v1:
* Do not use a machine type, use a machine parameter.

Okay, it has been discussed already. I'd appreciate a brief recap all
the same.

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

[libvirt] [PATCH] blkdeviotune: trigger tunable event for blkdeviotune updates

With the blkdeviotune event this patch also fixes a bug that the updated
live values weren't saved to the live XML so they won't survive
restarting the libvirtd.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 include/libvirt/libvirt.h.in | 56 
 src/qemu/qemu_driver.c   | 54 ++
 2 files changed, 110 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 898f8b5..32930e2 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5262,6 +5262,61 @@ typedef void 
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
  */
 #define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA cputune.emulator_quota
 
+/**
+ * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_DISK:
+ *
+ * Macro represents the name of guest disk for which the values are updated,
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_DISK blkdeviotune.disk
+
+/**
+ * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_BYTES_SEC:
+ *
+ * Marco represents the total throughput limit in bytes per second,
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_BYTES_SEC 
blkdeviotune.total_bytes_sec
+
+/**
+ * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_BYTES_SEC:
+ *
+ * Marco represents the read throughput limit in bytes per second,
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_BYTES_SEC 
blkdeviotune.read_bytes_sec
+
+/**
+ * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_BYTES_SEC:
+ *
+ * Macro represents the write throughput limit in bytes per second,
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_BYTES_SEC 
blkdeviotune.write_bytes_sec
+
+/**
+ * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_IOPS_SEC:
+ *
+ * Macro represents the total I/O operations per second,
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_IOPS_SEC 
blkdeviotune.total_iops_sec
+
+/**
+ * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_IOPS_SEC:
+ *
+ * Macro represents the read I/O operations per second,
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_IOPS_SEC 
blkdeviotune.read_iops_sec
+
+/**
+ * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_IOPS_SEC:
+ *
+ * Macro represents the write I/O operations per second,
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_IOPS_SEC 
blkdeviotune.write_iops_sec
 
 /**
  * virConnectDomainEventTunableCallback:
@@ -5277,6 +5332,7 @@ typedef void 
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
  *
  * Currently supported name spaces:
  *  cputune.*
+ *  blkdeviotune.*
  *
  * The callback signature to use when registering for an event of type
  * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny()
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d1a0657..17a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16274,6 +16274,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
 bool set_iops = false;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
+virObjectEventPtr event = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxparams = 0;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -16315,6 +16319,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
 persistentDef)  0)
 goto endjob;
 
+if (virTypedParamsAddString(eventParams, eventNparams, eventMaxparams,
+VIR_DOMAIN_EVENT_BLKDEVIOTUNE_DISK, disk)  0)
+goto endjob;
+
 for (i = 0; i  nparams; i++) {
 virTypedParameterPtr param = params[i];
 
@@ -16328,26 +16336,56 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
 if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) {
 info.total_bytes_sec = param-value.ul;
 set_bytes = true;
+if (virTypedParamsAddULLong(eventParams, eventNparams,
+eventMaxparams,
+
VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_BYTES_SEC,
+param-value.ul)  0)
+goto endjob;
 } else if (STREQ(param-field,
  VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) {
 info.read_bytes_sec = param-value.ul;
 set_bytes = true;
+if (virTypedParamsAddULLong(eventParams, eventNparams,
+eventMaxparams,
+
VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_BYTES_SEC,
+param-value.ul)  0)
+goto endjob;
 } else if (STREQ(param-field,
  VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) {
 info.write_bytes_sec = param-value.ul;

Re: [libvirt] [PATCHv2 4/4] storage: Improve error message when traversing backing chains

2014-09-24 Thread Peter Krempa

On 09/23/14 22:35, John Ferlan wrote:
 
 
 On 09/18/2014 05:54 AM, Peter Krempa wrote:
 Report also the name of the parent file and uid/gid used to access it to
 help debugging broken storage configurations.
 ---
  src/storage/storage_driver.c | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)

 
 ACK
 

Thanks. I've fixed the nits you've pointed out and pushed this series.

Peter




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

Re: [libvirt] [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm

2014-09-24 Thread Alex Bligh

Markus,

On 24 Sep 2014, at 09:05, Markus Armbruster arm...@redhat.com wrote:

Alex Bligh a...@alex.org.uk writes:

Forgive me if this has been discussed already: why not simply a separate
machine type pc-1.0-qemu-kvm?

That's what v2 of the patch set does (and I prefer v2). However, mst
wanted it done this way.

Three aproaches are taken:

This part fires only for pc-1.0, because it's in
pc_early_init_pci_1_0().

Yes, intentional. I should have noted that. That's because
qemu-kvm-migration=on the VRAM change was designed to
work only with pc-1.0. I haven't looked at trying to make
other (older) qemu-kvm machine migrations work, but (with the stuff
below), I would guess it would work with the appropriate
command line options for VRAM size.

Obviously I could put early init elsewhere.

These parts apply to all machine types, don't they?

They apply only when qemu-kvm-migration=on is selected, but to
any machine type; however machine types newer than pc-1.0 will
be exporting v3 I think anyway.

Changes since v1:
* Do not use a machine type, use a machine parameter.

Okay, it has been discussed already. I'd appreciate a brief recap all
the same.

See above. I preferred the machine type. But I got to learn more about
QOM on the way :-)

--
Alex Bligh

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

Re: [libvirt] [PATCH] Fix bug with loading bridge name for active domain during libvirtd start

2014-09-24 Thread Peter Krempa

On 09/18/14 15:31, Pavel Hrdina wrote:
 If you have a bridge network in running domain and libvirtd is restarted
 the information about host bridge interface is lost from live xml.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140085
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
  src/conf/domain_conf.c | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 3ccec1c..fa4166c 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -6837,6 +6837,10 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
  goto error;
  }
  VIR_FREE(class_id);
 +} else if (actual-type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
 +char *brname = virXPathString(string(./source/@bridge), ctxt);
 +if (brname)
 +actual-data.bridge.brname = brname;
  }
  
  bandwidth_node = virXPathNode(./bandwidth, ctxt);
 

Parsers for other network types error out if the requested field is not
present. Also the intermediate variable shouldn't be necessary.

I'd like to see a version with the error before I ACK it, although the
idea is ok.

Peter



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

[libvirt] [PATCH v2] Fix bug with loading bridge name for active domain during libvirtd start

If you have a bridge network in running domain and libvirtd is restarted
the information about host bridge interface is lost from live xml.

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

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---

changes in v2:
 - added error message if bridge name is missing.

 src/conf/domain_conf.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9cc118c..0a7d0b8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6850,6 +6850,15 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
 goto error;
 }
 VIR_FREE(class_id);
+} else if (actual-type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+char *brname = virXPathString(string(./source/@bridge), ctxt);
+if (!brname) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Missing source element with bridge name in 
+ interface's actual element));
+goto error;
+}
+actual-data.bridge.brname = brname;
 }
 
 bandwidth_node = virXPathNode(./bandwidth, ctxt);
-- 
1.8.5.5

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

Re: [libvirt] [PATCH v2] Fix bug with loading bridge name for active domain during libvirtd start

2014-09-24 Thread Peter Krempa

On 09/24/14 11:36, Pavel Hrdina wrote:
 If you have a bridge network in running domain and libvirtd is restarted
 the information about host bridge interface is lost from live xml.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140085
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
 
 changes in v2:
  - added error message if bridge name is missing.
 
  src/conf/domain_conf.c | 9 +
  1 file changed, 9 insertions(+)
 

ACK




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

Re: [libvirt] [PATCH v2] Fix bug with loading bridge name for active domain during libvirtd start


On 09/24/2014 11:45 AM, Peter Krempa wrote:

On 09/24/14 11:36, Pavel Hrdina wrote:

If you have a bridge network in running domain and libvirtd is restarted
the information about host bridge interface is lost from live xml.

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

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---

changes in v2:
  - added error message if bridge name is missing.

  src/conf/domain_conf.c | 9 +
  1 file changed, 9 insertions(+)



ACK




Thanks, pushed.

Pavel

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

Re: [libvirt] [RFC PATCH] qcow2: Fix race in cache invalidation

2014-09-24 Thread Kevin Wolf

Am 23.09.2014 um 10:47 hat Alexey Kardashevskiy geschrieben:
 On 09/19/2014 06:47 PM, Kevin Wolf wrote: Am 16.09.2014 um 14:59 hat Paolo 
 Bonzini geschrieben:
  Il 16/09/2014 14:52, Kevin Wolf ha scritto:
  Yes, that's true. We can't fix this problem in qcow2, though, because
  it's a more general one.  I think we must make sure that
  bdrv_invalidate_cache() doesn't yield.
 
  Either by forbidding to run bdrv_invalidate_cache() in a coroutine and
  moving the problem to the caller (where and why is it even called from a
  coroutine?), or possibly by creating a new coroutine for the driver
  callback and running that in a nested event loop that only handles
  bdrv_invalidate_cache() callbacks, so that the NBD server doesn't get a
  chance to process new requests in this thread.
 
  Incoming migration runs in a coroutine (the coroutine entry point is
  process_incoming_migration_co).  But everything after qemu_fclose() can
  probably be moved into a separate bottom half, so that it gets out of
  coroutine context.
  
  Alexey, you should probably rather try this (and add a bdrv_drain_all()
  in bdrv_invalidate_cache) than messing around with qcow2 locks. This
  isn't a problem that can be completely fixed in qcow2.
 
 
 Ok. Tried :) Not very successful though. The patch is below.
 
 Is that the correct bottom half? When I did it, I started getting crashes
 in various sport on accesses to s-l1_cache which is NULL after qcow2_close.
 Normally the code would check s-l1_size and then use but they are out of 
 sync.

No, that's not the place we were talking about.

What Paolo meant is that in process_incoming_migration_co(), you can
split out the final part that calls bdrv_invalidate_cache_all() into a
BH (you need to move everything until the end of the function into the
BH then). His suggestion was to move everything below the qemu_fclose().

 So I clear it in qcow2_close(). This allowed migrated guest to work and not
 to crash until I shut it down when it aborted at HERE IT FAILS ON SHUTDOWN.
 
 Here I realized I am missing something in this picture again, what is it?

The problem with your patch seems to be that you close the image and
then let the VM access the image before it is reopened in the BH. That
can't work out well. This is why it's important that the vm_start() call
is in the BH, too.

  void bdrv_invalidate_cache_all(Error **errp)
 diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
 index 904f6b1..59ff48c 100644
 --- a/block/qcow2-cache.c
 +++ b/block/qcow2-cache.c
 @@ -259,7 +259,7 @@ static int qcow2_cache_find_entry_to_replace(Qcow2Cache 
 *c)
  if (min_index == -1) {
  /* This can't happen in current synchronous code, but leave the check
   * here as a reminder for whoever starts using AIO with the cache */
 -abort();
 +abort(); //  HERE IT FAILS ON SHUTDOWN
  }
  return min_index;
  }

It's a weird failure mode anyway...

Kevin

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

[libvirt] [PATCH 2/6] network: set interface actual trustGuestRxFilters from network/portgroup

As is done with other items such as vlan, virtualport, and bandwidth,
set the actual trustGuestRxFilters value to be used by a domain
interface according to a merge of the same attribute in the interface,
portgroup, and network in use. the interface setting always takes
precedence (if specified), followed by portgroup, and finally the
setting in the network is used if it's not specified in the interface
or portgroup.
---
 src/network/bridge_driver.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 979fb13..548e354 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3794,6 +3794,17 @@ networkAllocateActualDevice(virDomainDefPtr dom,
 if (vlan  virNetDevVlanCopy(iface-data.network.actual-vlan, vlan)  0)
 goto error;
 
+if (iface-trustGuestRxFilters != VIR_TRISTATE_BOOL_ABSENT)
+   iface-data.network.actual-trustGuestRxFilters
+  = iface-trustGuestRxFilters;
+else if (portgroup 
+ portgroup-trustGuestRxFilters != VIR_TRISTATE_BOOL_ABSENT)
+   iface-data.network.actual-trustGuestRxFilters
+  = portgroup-trustGuestRxFilters;
+else if (netdef-trustGuestRxFilters != VIR_TRISTATE_BOOL_ABSENT)
+   iface-data.network.actual-trustGuestRxFilters
+  = netdef-trustGuestRxFilters;
+
 if ((netdef-forward.type == VIR_NETWORK_FORWARD_NONE) ||
 (netdef-forward.type == VIR_NETWORK_FORWARD_NAT) ||
 (netdef-forward.type == VIR_NETWORK_FORWARD_ROUTE)) {
-- 
1.9.3

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

[libvirt] [PATCH 3/6] util: define virNetDevRxFilter and basic utility functions

This same structure will be used to retrieve RX filter info for
interfaces on the host via netlink messages, and RX filter info for
interfaces on the guest via the qemu query-rx-filter command.
---
 src/libvirt_private.syms |  8 +++
 src/util/virnetdev.c | 40 +
 src/util/virnetdev.h | 57 +++-
 3 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bb2b9a3..e5723d2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1595,6 +1595,14 @@ virNetDevReplaceMacAddress;
 virNetDevReplaceNetConfig;
 virNetDevRestoreMacAddress;
 virNetDevRestoreNetConfig;
+virNetDevRxFilterFree;
+virNetDevRxFilterMulticastModeTypeFromString;
+virNetDevRxFilterMulticastModeTypeToString;
+virNetDevRxFilterNew;
+virNetDevRxFilterUnicastModeTypeFromString;
+virNetDevRxFilterUnicastModeTypeToString;
+virNetDevRxFilterVlanModeTypeFromString;
+virNetDevRxFilterVlanModeTypeToString;
 virNetDevSetIPv4Address;
 virNetDevSetMAC;
 virNetDevSetMTU;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 8815e18..dd1f530 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1932,3 +1932,43 @@ virNetDevGetLinkInfo(const char *ifname,
 return 0;
 }
 #endif /* defined(__linux__) */
+
+
+VIR_ENUM_IMPL(virNetDevRxFilterUnicastMode,
+  VIR_NETDEV_RX_FILTER_UNICAST_MODE_LAST,
+  none,
+  normal);
+
+VIR_ENUM_IMPL(virNetDevRxFilterMulticastMode,
+  VIR_NETDEV_RX_FILTER_MULTICAST_MODE_LAST,
+  none,
+  normal);
+
+VIR_ENUM_IMPL(virNetDevRxFilterVlanMode,
+  VIR_NETDEV_RX_FILTER_VLAN_MODE_LAST,
+  none,
+  normal);
+
+
+virNetDevRxFilterPtr
+virNetDevRxFilterNew(void)
+{
+virNetDevRxFilterPtr filter;
+
+if (VIR_ALLOC(filter)  0)
+return NULL;
+return filter;
+}
+
+
+void
+virNetDevRxFilterFree(virNetDevRxFilterPtr filter)
+{
+if (filter) {
+VIR_FREE(filter-name);
+VIR_FREE(filter-unicast.table);
+VIR_FREE(filter-multicast.table);
+VIR_FREE(filter-vlan.table);
+VIR_FREE(filter);
+}
+}
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 69e365e..307871c 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2013 Red Hat, Inc.
+ * Copyright (C) 2007-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -37,6 +37,57 @@ typedef struct ifreq virIfreq;
 typedef void virIfreq;
 # endif
 
+typedef enum {
+   VIR_NETDEV_RX_FILTER_UNICAST_MODE_NONE = 0,
+   VIR_NETDEV_RX_FILTER_UNICAST_MODE_NORMAL,
+
+   VIR_NETDEV_RX_FILTER_UNICAST_MODE_LAST
+} virNetDevRxFilterUnicastMode;
+VIR_ENUM_DECL(virNetDevRxFilterUnicastMode)
+
+typedef enum {
+   VIR_NETDEV_RX_FILTER_MULTICAST_MODE_NONE = 0,
+   VIR_NETDEV_RX_FILTER_MULTICAST_MODE_NORMAL,
+
+   VIR_NETDEV_RX_FILTER_MULTICAST_MODE_LAST
+} virNetDevRxFilterMulticastMode;
+VIR_ENUM_DECL(virNetDevRxFilterMulticastMode)
+
+typedef enum {
+   VIR_NETDEV_RX_FILTER_VLAN_MODE_NONE = 0,
+   VIR_NETDEV_RX_FILTER_VLAN_MODE_NORMAL,
+
+   VIR_NETDEV_RX_FILTER_VLAN_MODE_LAST
+} virNetDevRxFilterVlanMode;
+VIR_ENUM_DECL(virNetDevRxFilterVlanMode)
+
+typedef struct _virNetDevRxFilter virNetDevRxFilter;
+typedef virNetDevRxFilter *virNetDevRxFilterPtr;
+struct _virNetDevRxFilter {
+char *name; /* the alias used by qemu, *not* name used by guest */
+virMacAddr mac;
+bool promiscuous;
+bool broadcastAllowed;
+
+struct {
+int mode; /* enum virNetDevRxFilterUnicastMode */
+bool overflow;
+virMacAddrPtr table;
+size_t nTable;
+} unicast;
+struct {
+int mode; /* enum virNetDevRxFilterMulticastMode */
+bool overflow;
+virMacAddrPtr table;
+size_t nTable;
+} multicast;
+struct {
+int mode; /* enum virNetDevRxFilterVlanMode */
+unsigned int *table;
+size_t nTable;
+} vlan;
+};
+
 int virNetDevSetupControl(const char *ifname,
   virIfreq *ifr)
 ATTRIBUTE_RETURN_CHECK;
@@ -150,4 +201,8 @@ int virNetDevGetLinkInfo(const char *ifname,
  virInterfaceLinkPtr lnk)
 ATTRIBUTE_NONNULL(1);
 
+virNetDevRxFilterPtr virNetDevRxFilterNew(void)
+   ATTRIBUTE_RETURN_CHECK;
+void virNetDevRxFilterFree(virNetDevRxFilterPtr filter);
+
 #endif /* __VIR_NETDEV_H__ */
-- 
1.9.3

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

[libvirt] [PATCH 4/6] qemu: qemuMonitorQueryRxFilter - retrieve guest netdev rx-filter

This function can be called at any time to get the current status of a
guest's network device rx-filter. In particular it is useful to call
after libvirt recieves a NIC_RX_FILTER_CHANGED event - this event only
tells you that something has changed in the rx-filter, the details are
retrieved with the query-rx-filter monitor command (only available in
the json monitor). The commend sent to the qemu monitor looks like this:

  {execute:query-rx-filter, arguments: {name:net2} }'

and the results will look something like this:

{
return: [
{
promiscuous: false,
name: net2,
main-mac: 52:54:00:98:2d:e3,
unicast: normal,
vlan: normal,
vlan-table: [
42,
0
],
unicast-table: [

],
multicast: normal,
multicast-overflow: false,
unicast-overflow: false,
multicast-table: [
33:33:ff:98:2d:e3,
01:80:c2:00:00:21,
01:00:5e:00:00:fb,
33:33:ff:98:2d:e2,
01:00:5e:00:00:01,
33:33:00:00:00:01
],
broadcast-allowed: false
}
],
id: libvirt-14
}

This is all parsed from JSON into a virNetDevRxFilter object for
easier consumption. (unicast-table is usually empty, but is also an
array of mac addresses similar to multicast-table).

(NB: LIBNL_CFLAGS was added to tests/Makefile.am because virnetdev.h
now includes util/virnetlink.h, which includes netlink/msg.h when
appropriate. Without LIBNL_CFLAGS, gcc can't find that file (if
libnl/netlink isn't available, LIBNL_CFLAGS will be empty and
virnetlink.h won't try to include netlink/msg.h anyway).)
---
 src/qemu/qemu_monitor.c  |  26 ++
 src/qemu/qemu_monitor.h  |   4 +
 src/qemu/qemu_monitor_json.c | 215 +++
 src/qemu/qemu_monitor_json.h |   3 +
 tests/Makefile.am|   3 +
 5 files changed, 251 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c25f002..48cbe3e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2918,6 +2918,32 @@ int qemuMonitorRemoveNetdev(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias,
+ virNetDevRxFilterPtr *filter)
+{
+int ret = -1;
+VIR_DEBUG(mon=%p alias=%s filter=%p,
+  mon, alias, filter);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+   _(monitor must not be NULL));
+return -1;
+}
+
+
+VIR_DEBUG(mon=%p, alias=%s, mon, alias);
+
+if (mon-json)
+ret = qemuMonitorJSONQueryRxFilter(mon, alias, filter);
+else
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(query-rx-filter requires JSON monitor));
+return ret;
+}
+
+
 int qemuMonitorGetPtyPaths(qemuMonitorPtr mon,
virHashTablePtr paths)
 {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 6b91e29..c37e36f 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -31,6 +31,7 @@
 # include virbitmap.h
 # include virhash.h
 # include virjson.h
+# include virnetdev.h
 # include device_conf.h
 # include cpu/cpu.h
 
@@ -622,6 +623,9 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon,
 int qemuMonitorRemoveNetdev(qemuMonitorPtr mon,
 const char *alias);
 
+int qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias,
+ virNetDevRxFilterPtr *filter);
+
 int qemuMonitorGetPtyPaths(qemuMonitorPtr mon,
virHashTablePtr paths);
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a3d7c2c..58007e6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3194,6 +3194,221 @@ int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon,
 }
 
 
+static int
+qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
+  virNetDevRxFilterPtr *filter)
+{
+int ret = -1;
+const char *tmp;
+virJSONValuePtr returnArray, entry, table, element;
+int nTable;
+size_t i;
+virNetDevRxFilterPtr fil = virNetDevRxFilterNew();
+
+if (!(returnArray = virJSONValueObjectGet(msg, return))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(query-rx-filter reply was missing return data));
+goto cleanup;
+}
+if (returnArray-type != VIR_JSON_TYPE_ARRAY) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(query-rx-filter return data was not an array));
+goto cleanup;
+}
+if (!(entry = virJSONValueArrayGet(returnArray, 0))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(query -rx-filter returne data missing array 
element));
+goto cleanup;
+}
+
+

[libvirt] [PATCH 5/6] qemu: setup infrastructure to handle NIC_RX_FILTER_CHANGED event

NIC_RX_FILTER_CHANGED is sent by qemu any time a NIC driver in the
guest modified the NIC's RX Filter (for example, if the MAC address of
the NIC is changed by the guest).

This patch doesn't do anything useful with that event; it just sets up
all the plumbing to get news of the event into a worker thread with
all proper locking/reference counting, and provide an easy place to
add in desired functionality.

For easy reference the next time a handler for a qemu event is
written, here is the sequence:

The handler in qemu_json_monitor.c calls

   qemu_json_monitor.c:qemuMonitorJSONHandleNicRxFilterChanged()

which calls

   qemu_monitor.c:qemuMonitorEmitNicRxFilterChanged()

which uses QEMU_MONITOR_CALLBACK() to call
mon-cb-domainNicRxFilterChanged(), ie:

   qemuProcessHandleNicRxFilterChanged()

which will queue an event QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED for
a worker thread to handle.

When the worker thread gets the event, it calls

   qemuProcessEventHandler()

which calls

   processNicRxFilterChangedEvent()

and *that* is where the actual work will be done (and any
event-specific memory allocated during qemuProcessHandleXXX() will be
freed) - it is the middle of this function where functionality behind
the event will be added in the next patch; for now there is just a
VIR_DEBUG() to log the event.
---
 src/qemu/qemu_domain.h   |  1 +
 src/qemu/qemu_driver.c   | 55 
 src/qemu/qemu_monitor.c  | 13 +++
 src/qemu/qemu_monitor.h  |  7 ++
 src/qemu/qemu_monitor_json.c | 17 ++
 src/qemu/qemu_process.c  | 42 +
 6 files changed, 135 insertions(+)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 845d3c7..ad45a66 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -195,6 +195,7 @@ typedef enum {
 QEMU_PROCESS_EVENT_WATCHDOG = 0,
 QEMU_PROCESS_EVENT_GUESTPANIC,
 QEMU_PROCESS_EVENT_DEVICE_DELETED,
+QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED,
 
 QEMU_PROCESS_EVENT_LAST
 } qemuProcessEventType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 543de79..64f1d45 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4140,6 +4140,58 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
 }
 
 
+static void
+processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   char *devAlias)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virDomainDeviceDef dev;
+virDomainNetDefPtr def;
+
+VIR_DEBUG(Received NIC_RX_FILTER_CHANGED event for device %s 
+  from domain %p %s,
+  devAlias, vm, vm-def-name);
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+VIR_DEBUG(Domain is not running);
+goto endjob;
+}
+
+if (virDomainDefFindDevice(vm-def, devAlias, dev, true)  0) {
+VIR_WARN(NIC_RX_FILTER_CHANGED event received for 
+ non-existent device %s in domain %s,
+ devAlias, vm-def-name);
+goto endjob;
+}
+if (dev.type != VIR_DOMAIN_DEVICE_NET) {
+VIR_WARN(NIC_RX_FILTER_CHANGED event received for 
+ non-network device %s in domain %s,
+ devAlias, vm-def-name);
+goto endjob;
+}
+def = dev.data.net;
+
+/* handle the event - send query-rx-filter and respond to it. */
+
+VIR_DEBUG(process NIC_RX_FILTER_CHANGED event for network 
+  device %s in domain %s, def-info.alias, vm-def-name);
+
+ endjob:
+/* We had an extra reference to vm before starting a new job so ending the
+ * job is guaranteed not to remove the last reference.
+ */
+ignore_value(qemuDomainObjEndJob(driver, vm));
+
+ cleanup:
+VIR_FREE(devAlias);
+virObjectUnref(cfg);
+}
+
+
 static void qemuProcessEventHandler(void *data, void *opaque)
 {
 struct qemuProcessEvent *processEvent = data;
@@ -4160,6 +4212,9 @@ static void qemuProcessEventHandler(void *data, void 
*opaque)
 case QEMU_PROCESS_EVENT_DEVICE_DELETED:
 processDeviceDeletedEvent(driver, vm, processEvent-data);
 break;
+case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
+processNicRxFilterChangedEvent(driver, vm, processEvent-data);
+break;
 case QEMU_PROCESS_EVENT_LAST:
 break;
 }
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 48cbe3e..a4661f8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1387,6 +1387,19 @@ qemuMonitorEmitDeviceDeleted(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorEmitNicRxFilterChanged(qemuMonitorPtr mon,
+  const char *devAlias)
+{
+int ret = -1;
+VIR_DEBUG(mon=%p, mon);
+
+QEMU_MONITOR_CALLBACK(mon, ret, domainNicRxFilterChanged, mon-vm, 
devAlias);
+
+

[libvirt] [PATCH 6/6] qemu: change macvtap device MAC address in response to NIC_RX_FILTER_CHANGED

This patch fills in the functionality of
processNicRxFilterChangedEvent().  It now checks if it is appropriate
to respond to the NIC_RX_FILTER_CHANGED event (based on device type
and configuration) and takes appropriate action. Currently it checks
if the guest interface has been configured with
trustGuestRxFilters='yes', and if the host side device is macvtap. If
so, and the MAC address on the guest has changed, the MAC address of
the macvtap device is changed to match.

The result of this is that networking from the guest will continue to
work if the mac address of a macvtap-connected network device is
changed from within the guest, as long as trustGuestRxFilters='yes'
(previously changing the MAC address in the guest would break
networking).
---
 src/qemu/qemu_driver.c | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 64f1d45..7801d91 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4146,8 +4146,13 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
char *devAlias)
 {
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+qemuDomainObjPrivatePtr priv = vm-privateData;
 virDomainDeviceDef dev;
 virDomainNetDefPtr def;
+virNetDevRxFilterPtr filter = NULL;
+virMacAddr oldMAC;
+char newMacStr[VIR_MAC_STRING_BUFLEN];
+int ret;
 
 VIR_DEBUG(Received NIC_RX_FILTER_CHANGED event for device %s 
   from domain %p %s,
@@ -4175,11 +4180,55 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
 }
 def = dev.data.net;
 
+if (!virDomainNetGetActualTrustGuestRxFilters(def)) {
+VIR_WARN(ignore NIC_RX_FILTER_CHANGED event for network 
+  device %s in domain %s,
+  def-info.alias, vm-def-name);
+/* not sending query-rx-filter will also suppress any
+ * further NIC_RX_FILTER_CHANGED events for this device
+ */
+goto endjob;
+}
+
 /* handle the event - send query-rx-filter and respond to it. */
 
 VIR_DEBUG(process NIC_RX_FILTER_CHANGED event for network 
   device %s in domain %s, def-info.alias, vm-def-name);
 
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorQueryRxFilter(priv-mon, devAlias, filter);
+qemuDomainObjExitMonitor(driver, vm);
+if (ret  0)
+goto endjob;
+
+virMacAddrFormat(filter-mac, newMacStr);
+
+if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) {
+
+/* For macvtap connections, set the macvtap device's MAC
+ * address to match that of the guest device.
+ */
+
+if (virNetDevGetMAC(def-ifname, oldMAC)  0) {
+VIR_WARN(Couldn't get current MAC address of device %s 
+ while responding to NIC_RX_FILTER_CHANGED,
+ def-ifname);
+goto endjob;
+}
+
+if (virMacAddrCmp(oldMAC, filter-mac)) {
+/* set new MAC address from guest to associated macvtap device */
+if (virNetDevSetMAC(def-ifname, filter-mac)  0) {
+VIR_WARN(Couldn't set new MAC address %s to device %s 
+ while responding to NIC_RX_FILTER_CHANGED,
+ newMacStr, def-ifname);
+} else {
+VIR_DEBUG(device %s MAC address set to %s,
+  def-ifname, newMacStr);
+}
+}
+}
+
  endjob:
 /* We had an extra reference to vm before starting a new job so ending the
  * job is guaranteed not to remove the last reference.
@@ -4187,6 +4236,7 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
 ignore_value(qemuDomainObjEndJob(driver, vm));
 
  cleanup:
+virNetDevRxFilterFree(filter);
 VIR_FREE(devAlias);
 virObjectUnref(cfg);
 }
-- 
1.9.3

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

[libvirt] [PATCH 1/6] conf: add trustGuestRxFilters attribute to network and domain interface

This new attribute will control whether or not libvirt will pay
attention to guest notifications about changes to network device mac
addresses and receive filters. The default for this is 'no' (for
security reasons). If it is set to 'yes' *and* the specified device
model and connection support it (currently only macvtap+virtio) then
libvirt will watch for NIC_RX_FILTER_CHANGED events, and when it
receives one, it will issue a query-rx-filter command, retrieve the
result, and modify the host-side macvtap interface's mac address and
unicast/multicast filters accordingly.

The functionality behind this attribute will be in a later patch. This
patch merely adds the attribute to the top-level of a domain's
interface as well as to network and portgroup, and adds
documentation and schema/xml2xml tests. Rather than adding even more
test files, I've just added the net attribute in various applicable
places of existing test files.
---
 docs/formatdomain.html.in  | 38 
 docs/formatnetwork.html.in | 28 +--
 docs/schemas/domaincommon.rng  |  5 +++
 docs/schemas/network.rng   | 10 ++
 src/conf/domain_conf.c | 42 ++
 src/conf/domain_conf.h |  3 ++
 src/conf/network_conf.c| 35 ++
 src/conf/network_conf.h|  2 ++
 src/libvirt_private.syms   |  1 +
 tests/networkxml2xmlin/vepa-net.xml|  4 +--
 tests/networkxml2xmlout/vepa-net.xml   |  4 +--
 .../qemuxml2argv-net-virtio-network-portgroup.xml  |  4 +--
 12 files changed, 160 insertions(+), 16 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index eefdd5e..17e180d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3343,10 +3343,9 @@
 pre
   ...
   lt;devicesgt;
-lt;interface type='bridge'gt;
-  lt;source bridge='xenbr0'/gt;
-  lt;mac address='00:16:3e:5d:c7:9e'/gt;
-  lt;script path='vif-bridge'/gt;
+lt;interface type='direct' trustGuestRxFilters='yes'gt;
+  lt;source dev='eth0'/gt;
+  lt;mac address='52:54:00:5d:c7:9e'/gt;
   lt;boot order='1'/gt;
   lt;rom bar='off'/gt;
 lt;/interfacegt;
@@ -3356,8 +3355,21 @@
 p
   There are several possibilities for specifying a network
   interface visible to the guest.  Each subsection below provides
-  more details about common setup options.  Additionally,
-  each codelt;interfacegt;/code element has an
+  more details about common setup options.
+/p
+p
+  libvirt allows specifying when the host should trust reports
+  from the guest of changes to the interface mac address and
+  receive filters by setting the codetrustGuestRxFilters/code
+  attribute to codeyes/codespan class=sinceSince
+  1.2.9/span. codetrustGuestRxFilters/code defaults
+  to codeno/code for security reasons, and support depends on
+  the guest network device driver as well as the type of
+  connection on the host - currently it is only supported for the
+  virtio driver, and for macvtap connections on the host.
+/p
+p
+  Each codelt;interfacegt;/code element has an
   optional codelt;addressgt;/code sub-element that can tie
   the interface to a particular pci slot, with
   attribute codetype='pci'/code
@@ -3589,6 +3601,18 @@
   being the default mode. The individual modes cause the delivery of
   packets to behave as follows:
 /p
+p
+  If the model type is set to codevirtio/code and
+  interface's codetrustGuestRxFilters/code attribute is set
+  to codeyes/code, changes made to the interface mac address,
+  unicast/multicast receive filters, and vlan settings in the
+  guest will be monitored and propogated to the associated macvtap
+  device on the host. span class=sinceSince
+  1.2.9/span. If codetrustGuestRxFilters/code is not set, or
+  is not supported for the device model in use, an attempted
+  change to the mac address originating from the guest side will
+  result in a non-working network connection.
+/p
 
 dl
   dtcodevepa/code/dt
@@ -3621,7 +3645,7 @@
   ...
   lt;devicesgt;
 ...
-lt;interface type='direct'gt;
+lt;interface type='direct' trustGuestRxFilters='no'gt;
   lt;source dev='eth0' mode='vepa'/gt;
 lt;/interfacegt;
   lt;/devicesgt;
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 1a8ad8e..ff73952 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -35,7 +35,7 @@
 /p
 
 pre
-  lt;network ipv6='yes'gt;
+  lt;network ipv6='yes' trustGuestRxFilters='no'gt;
 lt;namegt;defaultlt;/namegt;
 lt;uuidgt;3e3fce45-4f53-4fa7-bb32-11f34168b82blt;/uuidgt;
 .../pre
@@ -60,6 +60,16

[libvirt] [PATCH 0/6] handle NIC_RX_FILTER_CHANGED events from qemu

These patches set up an event handler for qemu's NIC_RX_FILTER_CHANGED
event, which is sent whenever a guest makes a change to a network
device's unicast/multicast filter, vlan table, or MAC address.

The handler checks if it is appropriate to respond to the
NIC_RX_FILTER_CHANGED event (based on device type and configuration)
and takes appropriate action. Currently it checks if the guest
interface has been configured with trustGuestRxFilters='yes' (defaults
to 'no' for security reasons), and if the host side device is
macvtap. If so, and the MAC address on the guest has changed, the MAC
address of the macvtap device is changed to match.

The result of this is that networking from the guest will continue to
work if the mac address of a macvtap-connected network device is
changed from within the guest, as long as trustGuestRxFilters='yes'
(previously changing the MAC address in the guest would break
networking).

I still need to add code to compare the old and new unicast and
multicast lists and program the filters in the macvtap to match the
guest, and to check for a non-empty vlan table and handle that
(currently that means just setting promiscuous mode on the macvtap),
but that can come in a followup series.

Laine Stump (6):
  conf: add trustGuestRxFilters attribute to network and domain
interface
  network: set interface actual trustGuestRxFilters from
network/portgroup
  util: define virNetDevRxFilter and basic utility functions
  qemu: qemuMonitorQueryRxFilter - retrieve guest netdev rx-filter
  qemu: setup infrastructure to handle NIC_RX_FILTER_CHANGED event
  qemu: change macvtap device MAC address in response to
NIC_RX_FILTER_CHANGED

 docs/formatdomain.html.in  |  38 +++-
 docs/formatnetwork.html.in |  28 ++-
 docs/schemas/domaincommon.rng  |   5 +
 docs/schemas/network.rng   |  10 +
 src/conf/domain_conf.c |  42 
 src/conf/domain_conf.h |   3 +
 src/conf/network_conf.c|  35 
 src/conf/network_conf.h|   2 +
 src/libvirt_private.syms   |   9 +
 src/network/bridge_driver.c|  11 +
 src/qemu/qemu_domain.h |   1 +
 src/qemu/qemu_driver.c | 105 ++
 src/qemu/qemu_monitor.c|  39 
 src/qemu/qemu_monitor.h|  11 +
 src/qemu/qemu_monitor_json.c   | 232 +
 src/qemu/qemu_monitor_json.h   |   3 +
 src/qemu/qemu_process.c|  42 
 src/util/virnetdev.c   |  40 
 src/util/virnetdev.h   |  57 -
 tests/Makefile.am  |   3 +
 tests/networkxml2xmlin/vepa-net.xml|   4 +-
 tests/networkxml2xmlout/vepa-net.xml   |   4 +-
 .../qemuxml2argv-net-virtio-network-portgroup.xml  |   4 +-
 23 files changed, 711 insertions(+), 17 deletions(-)

-- 
1.9.3

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

Re: [libvirt] [PATCH 1/6] Added qemu postcopy-active migration status

On Tue, Sep 23, 2014 at 16:09:56 +0200, Cristian Klein wrote:
 Signed-off-by: Cristian Klein cristian.kl...@cs.umu.se
 ---
  src/qemu/qemu_migration.c| 1 +
  src/qemu/qemu_monitor.c  | 2 +-
  src/qemu/qemu_monitor.h  | 1 +
  src/qemu/qemu_monitor_json.c | 3 ++-
  src/qemu/qemu_monitor_text.c | 3 ++-
  5 files changed, 7 insertions(+), 3 deletions(-)
 
...
 diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
 index 46d2782..a3c8aa5 100644
 --- a/src/qemu/qemu_monitor_text.c
 +++ b/src/qemu/qemu_monitor_text.c
 @@ -1458,7 +1458,8 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr 
 mon,
  goto cleanup;
  }
  
 -if (status-status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE) {
 +if (status-status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE ||
 +status-status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE) 
 {
  tmp = end + 1;

I don't think we need to change text monitor code since we won't be
talking with HMP to any version of QEMU that supports post-copy
migration. Otherwise the patch is just fine and we just need to wait for
post-copy to land in QEMU.

Jirka

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

Re: [libvirt] [PATCH] nodeinfo: fix nodeGetFreePages when max node is zero

2014-09-24 Thread Michal Privoznik


On 24.09.2014 07:45, Jincheng Miao wrote:

In nodeGetFreePages, if startCell is given by '0',
and the max node number is '0' too. The for-loop
wouldn't be executed.
So convert it to while-loop.

Before:

virsh freepages --cellno 0 --pagesize 4

error: internal error: no suitable info found

After:

virsh freepages --cellno 0 --pagesize 4

4KiB: 472637

Signed-off-by: Jincheng Miao jm...@redhat.com
---
  src/nodeinfo.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2459922..1fe190a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2042,18 +2042,19 @@ nodeGetFreePages(unsigned int npages,
  }

  lastCell = MIN(lastCell, startCell + cellCount);
+cell = startCell;

-for (cell = startCell; cell  lastCell; cell++) {
+do {
  for (i = 0; i  npages; i++) {
  unsigned int page_size = pages[i];
  unsigned int page_free;

-if (virNumaGetPageInfo(cell, page_size, 0, NULL, page_free)  0)
+if (virNumaGetPageInfo(cell++, page_size, 0, NULL, page_free)  0)
  goto cleanup;

  counts[ncounts++] = page_free;
  }
-}
+} while (cell  lastCell);

  if (!ncounts) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,



There's no need to switch to do-while loop. All what is needed is cell 
= lastCell in the for loop. I'm rewriting the patch and pushing. ACK.


Michal

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

Re: [libvirt] Is seems necessary to pass migratable=no/yes to qemu.

2014-09-24 Thread Ján Tomko

On 09/24/2014 05:28 AM, zhang bo wrote:
 The patch
 http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=de0aeafe9ce3eb414c8b5d3aa8995d776a2952de
 removes invtsc flag in the host-model CPU.
 
 I'm wondering, will it be better to pass args migratable=no/yes to qemu, 
 and let qemu complete the
 remaining work? As that qemu has checked whether it's necessary to use invtsc 
 or not.

The 'migratable' property is only for -cpu host (cpu mode='host-passthrough'
in libvirt).

For mode='host-model', libvirt detects the model and features of the host CPU
and passes it as -cpu model,+feat,+feat2,...
so we can't leave that to QEMU.

 --
 invtsc is available only if using: -cpu host,migratable=no,+invtsc.
 http://git.qemu.org/?p=qemu.git;a=commit;h=120eee7d1fdb2eba15766cfff7b9bcdc902690b4
 --
 
 There's another problem, if we do not pass migratable=no to qemu.
 Consider if we set host mode to pass-through
-
   cpu mode='host-passthrough'
   /cpu
-
 then the vm-def-cpu-features contains invtsc. however, qemu will 
 automatically remove this cpu flag
 as that migration=no is not passed to it. thus, the guest will not start 
 up. This problem is in fact
 caused by the patch:
 http://libvirt.org/git/?p=libvirt.git;a=commit;h=fba6bc47cbcabbe08d42279691efb0dff3b9c997,
 it forbids guest domain to start up if the host has INVTSC while the 
 guest(qemu) does not.

Regardless of QEMU support for invtsc, I'm only able to start the domain,
restore or migration fails.

As far as I know, only 'invtsc' is the problematic feature, because it both
a) can appear in the host CPU (so libvirt assumes -cpu host will add it)
b) is checked by qemuProcessVerifyGuestCPU (and libvirt complains when it's
not there)

For other features, we only add them to qemu command line and let qemu filter
out the unsupported ones.

-
 for (i = 0; def-cpu  i  def-cpu-nfeatures; i++) {
 virCPUFeatureDefPtr feature = def-cpu-features[i];
 
 if (feature-policy != VIR_CPU_FEATURE_REQUIRE)
 continue;
 
 if (STREQ(feature-name, invtsc) 
 !cpuHasFeature(guestcpu, feature-name)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(host doesn't support invariant TSC));
 goto cleanup;
 }
 }
 break;
--
 
 
 In conclusion:
 1 Will it better to pass args migratable=yes/no to qemu rather than doing 
 the mask-invtsc job in libvirt?
 2 If the guest has pass-through cpu mode, then it's unable to start up, 
 because qemu removes invtsc, and
 vm-def-cpu-features has it. It seems a BUG.
 

I think the simplest fix for host-passthrough would be to apply the same
filter host-model has.

But since using invtsc with host-passthrough requires both +invtsc and
migratable=no, so we'd need to either add a 'migratable' option to
host-passthrough (this would skip the filter and add migratable=on), or allow
fine-tuning the features for host-passthrough too.

Jan



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

Re: [libvirt] [PATCH] nodeinfo: fix nodeGetFreePages when max node is zero

2014-09-24 Thread Jincheng Miao



On 09/24/2014 07:40 PM, Michal Privoznik wrote:

On 24.09.2014 07:45, Jincheng Miao wrote:

In nodeGetFreePages, if startCell is given by '0',
and the max node number is '0' too. The for-loop
wouldn't be executed.
So convert it to while-loop.

Before:

virsh freepages --cellno 0 --pagesize 4

error: internal error: no suitable info found

After:

virsh freepages --cellno 0 --pagesize 4

4KiB: 472637

Signed-off-by: Jincheng Miao jm...@redhat.com
---
  src/nodeinfo.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2459922..1fe190a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2042,18 +2042,19 @@ nodeGetFreePages(unsigned int npages,
  }

  lastCell = MIN(lastCell, startCell + cellCount);
+cell = startCell;

-for (cell = startCell; cell  lastCell; cell++) {
+do {
  for (i = 0; i  npages; i++) {
  unsigned int page_size = pages[i];
  unsigned int page_free;

-if (virNumaGetPageInfo(cell, page_size, 0, NULL, 
page_free)  0)
+if (virNumaGetPageInfo(cell++, page_size, 0, NULL, 
page_free)  0)

  goto cleanup;

  counts[ncounts++] = page_free;
  }
-}
+} while (cell  lastCell);

  if (!ncounts) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,



There's no need to switch to do-while loop. All what is needed is cell 
= lastCell in the for loop. I'm rewriting the patch and pushing. ACK.


Wait, if change the for condition to 'cell = lastCell', and pass 
startCell == -1,
the for-loop will execute twice, and will overwrite counts[1] which is 
not allocated.




Michal


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

[libvirt] [PATCH] Fix MinGW build

When building on mingw the format string for long long/unsigned long
long have to be I64d/I64u instead of lld/llu.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---

I'm not sure if this is the right way to do it so sending it for review.

 examples/object-events/event-test.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/examples/object-events/event-test.c 
b/examples/object-events/event-test.c
index 9e09736..e90b590 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -476,6 +476,15 @@ myDomainEventTunableCallback(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 printf(%s EVENT: Domain %s(%d) tunable updated:\n,
__func__, virDomainGetName(dom), virDomainGetID(dom));
 
+#ifdef WIN32
+/* MinGW doesn't know the lld/llu so we have to use I64f/I64u instead. */
+# define LLD_FORMAT %I64d
+# define LLU_FORMAT %I64u
+#else /* WIN32 */
+# define LLD_FORMAT %lld
+# define LLU_FORMAT %llu
+#endif /* WIN32 */
+
 for (i = 0; i  nparams; i++) {
 switch (params[i].type) {
 case VIR_TYPED_PARAM_INT:
@@ -485,10 +494,10 @@ myDomainEventTunableCallback(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 printf(\t%s: %u\n, params[i].field, params[i].value.ui);
 break;
 case VIR_TYPED_PARAM_LLONG:
-printf(\t%s: %lld\n, params[i].field, params[i].value.l);
+printf(\t%s: LLD_FORMAT\n, params[i].field, params[i].value.l);
 break;
 case VIR_TYPED_PARAM_ULLONG:
-printf(\t%s: %llu\n, params[i].field, params[i].value.ul);
+printf(\t%s: LLU_FORMAT\n, params[i].field, 
params[i].value.ul);
 break;
 case VIR_TYPED_PARAM_DOUBLE:
 printf(\t%s: %g\n, params[i].field, params[i].value.d);
-- 
1.8.5.5

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

Re: [libvirt] [PATCH 2/6] Added public API to enable post-copy migration

On Tue, Sep 23, 2014 at 16:09:57 +0200, Cristian Klein wrote:
 Added a new `libvirt` migration flag `VIR_MIGRATE_POSTCOPY` and the
 necessary code to pass this flag to qemu as migration capability.
 
 Signed-off-by: Cristian Klein cristian.kl...@cs.umu.se
 ---
  include/libvirt/libvirt.h.in |  1 +
  src/libvirt.c|  7 +++
  src/qemu/qemu_migration.c| 43 +++
  src/qemu/qemu_migration.h|  3 ++-
  4 files changed, 53 insertions(+), 1 deletion(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 6371b7b..bdc33c6 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -1225,6 +1225,7 @@ typedef enum {
  VIR_MIGRATE_ABORT_ON_ERROR= (1  12), /* abort migration on I/O 
 errors happened during migration */
  VIR_MIGRATE_AUTO_CONVERGE = (1  13), /* force convergence */
  VIR_MIGRATE_RDMA_PIN_ALL  = (1  14), /* RDMA memory pinning */
 +VIR_MIGRATE_POSTCOPY  = (1  15), /* enable (but don't start) 
 post-copy */
  } virDomainMigrateFlags;
  
  
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 1a285ca..33aeafa 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -5265,6 +5265,7 @@ virDomainMigrateDirect(virDomainPtr domain,
   * automatically when supported).
   *   VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe.
   *   VIR_MIGRATE_OFFLINE Migrate offline
 + *   VIR_MIGRATE_POSTCOPY Enable (but don't start) post-copy
   *
   * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
   * Applications using the VIR_MIGRATE_PEER2PEER flag will probably
 @@ -5291,6 +5292,12 @@ virDomainMigrateDirect(virDomainPtr domain,
   * can use either VIR_MIGRATE_NON_SHARED_DISK or
   * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive.
   *
 + * If you want to enable post-copy migration you must set the
 + * VIR_MIGRATE_POSTCOPY flag. Once migration is active, you may
 + * start post-copy by calling virDomainMigrateStartPostCopy.
 + * When to start post-copy is entirely left to the user, libvirt
 + * only implements the necessary mechanism.
 + *
   * In either case it is typically only necessary to specify a
   * URI if the destination host has multiple interfaces and a
   * specific interface is required to transmit migration data.
 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index a5bd825..bd1f2d6 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -1799,6 +1799,45 @@ qemuMigrationSetOffline(virQEMUDriverPtr driver,
  
  
  static int
 +qemuMigrationSetPostCopy(virQEMUDriverPtr driver,
 +virDomainObjPtr vm,
 +qemuDomainAsyncJob job)

s/   // in the two lines above to fix indentation.

 +{
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +int ret;
 +
 +if (qemuDomainObjEnterMonitorAsync(driver, vm, job)  0)
 +return -1;
 +
 +ret = qemuMonitorGetMigrationCapability(
 +priv-mon,
 +QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY);

QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY is not defined anywhere in this
patchset. It seems you forgot to update qemu_monitor.[ch].

 +
 +if (ret  0) {
 +goto cleanup;
 +} else if (ret == 0) {
 +if (job == QEMU_ASYNC_JOB_MIGRATION_IN) {
 +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
 +   _(Post-copy migration is not supported by 
 + target QEMU binary));
 +} else {
 +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
 +   _(Post-copy migration is not supported by 
 + source QEMU binary));
 +}
 +ret = -1;
 +goto cleanup;
 +}
 +
 +ret = qemuMonitorSetMigrationCapability(
 +priv-mon,
 +QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY);
 +
 + cleanup:
 +qemuDomainObjExitMonitor(driver, vm);
 +return ret;
 +}
 +static int
  qemuMigrationSetCompression(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  qemuDomainAsyncJob job)
 @@ -3580,6 +3619,10 @@ qemuMigrationRun(virQEMUDriverPtr driver,
  if (flags  VIR_MIGRATE_RDMA_PIN_ALL 
  qemuMigrationSetPinAll(driver, vm,
 QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
 +
 +if (flags  VIR_MIGRATE_POSTCOPY 
 +qemuMigrationSetPostCopy(driver, vm,
 + QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
  goto cleanup;
  
  if (qemuDomainObjEnterMonitorAsync(driver, vm,

I'd expect similar thing would need to be done in the Prepare phase on
destination... However, if destination does not need to set the
capability, we at least need to check if destination QEMU supports it
and report failure from Prepare if it doesn't. And the
QEMU_ASYNC_JOB_MIGRATION_IN branch

Re: [libvirt] [PATCH 3/6] Added new public API virDomainMigrateStartPostCopy

On Tue, Sep 23, 2014 at 16:09:58 +0200, Cristian Klein wrote:
 The user first start migration using the `VIR_MIGRATE_POSTCOPY` flag,
 then calls `virDomainMigrateStartPostCopy` asynchronously to switch from
 pre-copy to post-copy.
 
 Signed-off-by: Cristian Klein cristian.kl...@cs.umu.se
 ---
  include/libvirt/libvirt.h.in |  2 ++
  src/driver.h |  4 
  src/libvirt.c| 37 +
  src/libvirt_public.syms  |  5 +
  4 files changed, 48 insertions(+)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index bdc33c6..eabedfa 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -1346,6 +1346,8 @@ int virDomainMigrateToURI3(virDomainPtr domain,
 unsigned int nparams,
 unsigned int flags);
  
 +int virDomainMigrateStartPostCopy (virDomainPtr domain);
 +
  int virDomainMigrateSetMaxDowntime (virDomainPtr domain,
  unsigned long long downtime,
  unsigned int flags);
 diff --git a/src/driver.h b/src/driver.h
 index bb748c4..6866ccd 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -1212,6 +1212,9 @@ typedef int
virDomainStatsRecordPtr **retStats,
unsigned int flags);
  
 +typedef int
 +(*virDrvDomainMigrateStartPostCopy)(virDomainPtr domain);
 +
  typedef struct _virDriver virDriver;
  typedef virDriver *virDriverPtr;
  
 @@ -1435,6 +1438,7 @@ struct _virDriver {
  virDrvNodeGetFreePages nodeGetFreePages;
  virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
  virDrvConnectGetAllDomainStats connectGetAllDomainStats;
 +virDrvDomainMigrateStartPostCopy domainMigrateStartPostCopy;
  };
  
  
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 33aeafa..e685da2 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -17803,6 +17803,43 @@ virDomainMigrateSetCompressionCache(virDomainPtr 
 domain,
  
  
  /**
 + * virDomainMigrateStartPostCopy:
 + * @domain: a domain object
 + *
 + * Starts post-copy migration. This function has to be called while
 + * migration (initially pre-copy) is in progress. The migration operation
 + * must be called with the VIR_MIGRATE_POSTCOPY flag.
 + *
 + * Returns 0 in case of success, -1 otherwise.
 + */
 +int
 +virDomainMigrateStartPostCopy(virDomainPtr domain)
 +{
 +virConnectPtr conn;
 +
 +VIR_DOMAIN_DEBUG(domain);
 +
 +virResetLastError();
 +
 +virCheckDomainReturn(domain, -1);
 +conn = domain-conn;
 +
 +virCheckReadOnlyGoto(conn-flags, error);
 +
 +if (conn-driver-domainMigrateStartPostCopy) {
 +if (conn-driver-domainMigrateStartPostCopy(domain)  0)
 +goto error;
 +return 0;
 +}
 +
 +virReportUnsupportedError();
 + error:
 +virDispatchError(conn);
 +return -1;
 +}
 +
 +
 +/**
   * virDomainMigrateSetMaxSpeed:
   * @domain: a domain object
   * @bandwidth: migration bandwidth limit in MiB/s
 diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
 index e1f013f..ea17a07 100644
 --- a/src/libvirt_public.syms
 +++ b/src/libvirt_public.syms
 @@ -679,4 +679,9 @@ LIBVIRT_1.2.8 {
  virDomainStatsRecordListFree;
  } LIBVIRT_1.2.7;
  
 +LIBVIRT_1.2.9 {
 +global:
 +virDomainMigrateStartPostCopy;
 +} LIBVIRT_1.2.8;
 +

You will need to change this section since post-copy won't make it in
1.2.9 (which freezes tomorrow)... Looks good otherwise.

Jirka

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

Re: [libvirt] [PATCH] nodeinfo: fix nodeGetFreePages when max node is zero

2014-09-24 Thread Michal Privoznik


On 24.09.2014 14:00, Jincheng Miao wrote:


On 09/24/2014 07:40 PM, Michal Privoznik wrote:

On 24.09.2014 07:45, Jincheng Miao wrote:

In nodeGetFreePages, if startCell is given by '0',
and the max node number is '0' too. The for-loop
wouldn't be executed.
So convert it to while-loop.

Before:

virsh freepages --cellno 0 --pagesize 4

error: internal error: no suitable info found

After:

virsh freepages --cellno 0 --pagesize 4

4KiB: 472637

Signed-off-by: Jincheng Miao jm...@redhat.com
---
  src/nodeinfo.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2459922..1fe190a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2042,18 +2042,19 @@ nodeGetFreePages(unsigned int npages,
  }

  lastCell = MIN(lastCell, startCell + cellCount);
+cell = startCell;

-for (cell = startCell; cell  lastCell; cell++) {
+do {
  for (i = 0; i  npages; i++) {
  unsigned int page_size = pages[i];
  unsigned int page_free;

-if (virNumaGetPageInfo(cell, page_size, 0, NULL,
page_free)  0)
+if (virNumaGetPageInfo(cell++, page_size, 0, NULL,
page_free)  0)
  goto cleanup;

  counts[ncounts++] = page_free;
  }
-}
+} while (cell  lastCell);

  if (!ncounts) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,



There's no need to switch to do-while loop. All what is needed is cell
= lastCell in the for loop. I'm rewriting the patch and pushing. ACK.


Wait, if change the for condition to 'cell = lastCell', and pass
startCell == -1,
the for-loop will execute twice, and will overwrite counts[1] which is
not allocated.


Oh, right. Seems like I'm brainwashed today. This is the diff I've 
forgot to 'git add':


diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 0a7642c..05eab6c 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2041,7 +2041,7 @@ nodeGetFreePages(unsigned int npages,
 goto cleanup;
 }

-lastCell = MIN(lastCell, startCell + cellCount);
+lastCell = MIN(lastCell, startCell + cellCount - 1);

 for (cell = startCell; cell = lastCell; cell++) {
 for (i = 0; i  npages; i++) {

Michal

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

Re: [libvirt] [PATCH 4/6] Added domainMigrateStartPostCopy in qemu driver

On Tue, Sep 23, 2014 at 16:09:59 +0200, Cristian Klein wrote:
 Signed-off-by: Cristian Klein cristian.kl...@cs.umu.se
 ---
  src/qemu/qemu_driver.c   | 44 
 
  src/qemu/qemu_monitor.c  | 19 +++
  src/qemu/qemu_monitor.h  |  2 ++
  src/qemu/qemu_monitor_json.c | 18 ++
  src/qemu/qemu_monitor_json.h |  1 +
  src/qemu/qemu_monitor_text.c | 11 +++
  src/qemu/qemu_monitor_text.h |  2 ++

No need to touch qemu_monitor_text.[ch] since we will only use QMP with
any QEMU that supports post-copy migration.

  7 files changed, 97 insertions(+)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index e73d4f9..02c9a3b 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -11861,6 +11861,49 @@ qemuDomainGetJobStats(virDomainPtr dom,
  }
  
  
 +static int qemuMigrateStartPostCopy(virDomainPtr dom)
 +{
 +virQEMUDriverPtr driver = dom-conn-privateData;
 +virDomainObjPtr vm;
 +int ret = -1;
 +qemuDomainObjPrivatePtr priv;
 +
 +if (!(vm = qemuDomObjFromDomain(dom)))
 +goto cleanup;
 +
 +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP)  0)
 +goto cleanup;
 +
 +if (!virDomainObjIsActive(vm)) {
 +virReportError(VIR_ERR_OPERATION_INVALID,
 +   %s, _(domain is not running));
 +goto endjob;
 +}
 +
 +priv = vm-privateData;
 +
 +if (priv-job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) {
 +virReportError(VIR_ERR_OPERATION_INVALID, %s,
 +   _(post-copy can only be started 
 +  while migration is in progress));

We forbid the use of tabs for indentation. Please, run make syntax-check
(in addition to make check) before sending patches to catch this type of
issues.

 +goto cleanup;
 +}
 +
 +VIR_DEBUG(Starting post-copy);
 +qemuDomainObjEnterMonitor(driver, vm);
 +ret = qemuMonitorMigrateStartPostCopy(priv-mon);
 +qemuDomainObjExitMonitor(driver, vm);
 +
 + endjob:
 +if (!qemuDomainObjEndJob(driver, vm))
 +vm = NULL;
 +
 + cleanup:
 +if (vm)
 +virObjectUnlock(vm);
 +return ret;
 +}
 +
  static int qemuDomainAbortJob(virDomainPtr dom)
  {
  virQEMUDriverPtr driver = dom-conn-privateData;
 @@ -18259,6 +18302,7 @@ static virDriver qemuDriver = {
  .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */
  .connectGetDomainCapabilities = qemuConnectGetDomainCapabilities, /* 
 1.2.7 */
  .connectGetAllDomainStats = qemuConnectGetAllDomainStats, /* 1.2.8 */
 +.domainMigrateStartPostCopy = qemuMigrateStartPostCopy, /* 1.2.9 */

This will need to be updated.

  };
  
  
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 14688bf..0b230cc 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -2384,6 +2384,25 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
  return ret;
  }
  
 +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon)
 +{
 +int ret;
 +VIR_DEBUG(mon=%p, mon);
 +
 +if (!mon) {
 +virReportError(VIR_ERR_INVALID_ARG, %s,
 +   _(monitor must not be NULL));
 +return -1;
 +}
 +
 +if (mon-json)
 +ret = qemuMonitorJSONMigrateStartPostCopy(mon);
 +else
 +ret = qemuMonitorTextMigrateStartPostCopy(mon);

Just return an error if !mon-json as other recent functions in
qemu_monitor.c do.

 +return ret;
 +}
 +
 +
  int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
  {
  int ret;
 diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
 index 587f779..97d980d 100644
 --- a/src/qemu/qemu_monitor.h
 +++ b/src/qemu/qemu_monitor.h
 @@ -539,6 +539,8 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
   unsigned int flags,
   const char *unixfile);
  
 +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon);
 +
  int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
  
  int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon,
 diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
 index e98962b..14e7f84 100644
 --- a/src/qemu/qemu_monitor_json.c
 +++ b/src/qemu/qemu_monitor_json.c
 @@ -2798,6 +2798,24 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
  return ret;
  }
  
 +int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon)
 +{
 +int ret;
 +virJSONValuePtr cmd = 
 qemuMonitorJSONMakeCommand(migrate-start-postcopy, NULL);
 +virJSONValuePtr reply = NULL;
 +if (!cmd)
 +return -1;

I think the following would be better to avoid the long line:

virJSONValuePtr cmd;

cmd = qemuMonitorJSONMakeCommand(migrate-start-postcopy, NULL);
if (!cmd)
...

 +
 +ret = qemuMonitorJSONCommand(mon, cmd, reply);
 +
 +if (ret == 0)
 +ret = qemuMonitorJSONCheckError(cmd, reply);
 +
 +virJSONValueFree(cmd);
 +virJSONValueFree(reply);
 +return

Re: [libvirt] [PATCH 5/6] Added domainMigrateStartPostCopy in remote driver

On Tue, Sep 23, 2014 at 16:10:00 +0200, Cristian Klein wrote:
 Signed-off-by: Cristian Klein cristian.kl...@cs.umu.se
 ---
  src/remote/remote_driver.c   |  1 +
  src/remote/remote_protocol.x | 12 +++-
  2 files changed, 12 insertions(+), 1 deletion(-)
 
 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
 index 75a3a7b..9a6a974 100644
 --- a/src/remote/remote_driver.c
 +++ b/src/remote/remote_driver.c
 @@ -8152,6 +8152,7 @@ static virDriver remote_driver = {
  .nodeGetFreePages = remoteNodeGetFreePages, /* 1.2.6 */
  .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 
 1.2.7 */
  .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */
 +.domainMigrateStartPostCopy = remoteDomainMigrateStartPostCopy, /* 1.2.9 
 */
  };
  
  static virNetworkDriver network_driver = {
 diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
 index a4ca0c3..c443636 100644
 --- a/src/remote/remote_protocol.x
 +++ b/src/remote/remote_protocol.x
 @@ -3090,6 +3090,10 @@ struct remote_connect_get_all_domain_stats_args {
  struct remote_connect_get_all_domain_stats_ret {
  remote_domain_stats_record retStatsREMOTE_DOMAIN_LIST_MAX;
  };
 +
 +struct remote_domain_migrate_start_post_copy_args {
 +remote_nonnull_domain dom;
 +};
  /*- Protocol. -*/
  
  /* Define the program number, protocol version and procedure numbers here. */
 @@ -5472,5 +5476,11 @@ enum remote_procedure {
   * @generate: both
   * @acl: domain:block_write
   */
 -REMOTE_PROC_DOMAIN_BLOCK_COPY = 345
 +REMOTE_PROC_DOMAIN_BLOCK_COPY = 345,
 +
 +/**
 + * @generate: both
 + * @acl: domain:migrate
 + */
 +REMOTE_PROC_DOMAIN_MIGRATE_START_POST_COPY = 346
  };

This is pretty straightforward and you could have just squashed this
patch into Added new public API virDomainMigrateStartPostCopy.

Jirka

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

Re: [libvirt] [PATCH 3/6] Added new public API virDomainMigrateStartPostCopy