Re: [libvirt] [libvirt-glib PATCHv5 7/7] gobject: Add wrapper for virNetworkGetDHCPLeases
On Thu, Jul 09, 2015 at 02:41:45PM +0100, Zeeshan Ali (Khattak) wrote: The answer was that magnitude of ugliness is irrelevant. Surely you know that this is not anything objective. To me a single #ifdef is ugly enough but seems it's not for you so how do you expect either of us to convince each other with arguments? If you really need something more concrete, I see at least 12 #ifdefs needed. Pretty ugly for my taste at least. I'm just trying to have a discussion about something concrete. If just one #ifdef was needed, I guess you'll bear with me if I try to convince you to go this way, with 12 #ifdef, you've got a more understandable standing in opposing this. Quite hard to make a decision with so few details ;) All needed details are avaiable and it's only a matter of taste and amount of care we want to give to distros. Both are subjective matters. Same deal as with the amount of #ifdef needed, if only EL6 has a too old libvirt, then you are in a much better position to convince me than if Debian stable and latest Ubuntu (making things up) have a too old libvirt. Ie let's know the exact tradeoffs we are discussing here from a technical point of view, maybe we won't have to agree to disagree and look for a tie breaker ;) Christophe pgpF6VZriEhbs.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Check static route collisions
Martin Kletzander (2): conf: Add getter for network routes network: Add another collision check into networkCheckRouteCollision src/conf/network_conf.c | 26 ++ src/conf/network_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/network/bridge_driver_linux.c | 29 + 4 files changed, 59 insertions(+) -- 2.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] network: Add another collision check into networkCheckRouteCollision
The comment above that function says: This function can be a lot more
exhaustive, ..., so let's be.
Check for collisions between routes in the system and static routes
being added explicitly from the route/ element of the network XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1094205
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Laine suggested moving networkCheckRouteCollision() into
networkAddRouteToBridge() and I haven't done that simply because we
can check it where it is now. It would also mean parsing the file,
which we don't want to parse anyway, multiple times or storing the
results and I don't think it's worth neither the time nor space
complexity.
src/network/bridge_driver_linux.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/src/network/bridge_driver_linux.c
b/src/network/bridge_driver_linux.c
index e394dafb2216..66e5902a7b6f 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -69,6 +69,7 @@ int networkCheckRouteCollision(virNetworkDefPtr def)
char iface[17], dest[128], mask[128];
unsigned int addr_val, mask_val;
virNetworkIpDefPtr ipdef;
+virNetworkRouteDefPtr routedef;
int num;
size_t i;
@@ -123,6 +124,34 @@ int networkCheckRouteCollision(virNetworkDefPtr def)
goto out;
}
}
+
+for (i = 0;
+ (routedef = virNetworkDefGetRouteByIndex(def, AF_INET, i));
+ i++) {
+
+virSocketAddr r_mask, r_addr;
+virSocketAddrPtr tmp_addr = virNetworkRouteDefGetAddress(routedef);
+int r_prefix = virNetworkRouteDefGetPrefix(routedef);
+
+if (!tmp_addr ||
+virSocketAddrMaskByPrefix(tmp_addr, r_prefix, r_addr) 0 ||
+virSocketAddrPrefixToNetmask(r_prefix, r_mask, AF_INET) 0)
+continue;
+
+if ((r_addr.data.inet4.sin_addr.s_addr == addr_val)
+(r_mask.data.inet4.sin_addr.s_addr == mask_val)) {
+char *addr_str = virSocketAddrFormat(r_addr);
+if (!addr_str)
+virResetLastError();
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Route address '%s' collides with one
+ that's in the system already),
+ NULLSTR(addr_str));
+VIR_FREE(addr_str);
+ret = -1;
+goto out;
+}
+}
}
out:
--
2.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 2/2] Fix cloning of raw, sparse volumes
On 07/03/2015 09:54 AM, Ján Tomko wrote:
From: Prerna Saxena pre...@linux.vnet.ibm.com
When virsh vol-clone is attempted on a raw file where capacity allocation,
the resulting cloned volume has a size that matches the virtual-size of
the parent; in place of matching its actual, disk size.
This patch fixes the cloned disk to have same _allocated_size_ as
the parent file from which it was cloned.
Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html
Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Ján Tomko jto...@redhat.com
---
src/storage/storage_backend.c | 23 ++-
src/storage/storage_driver.c | 5 -
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index c71545c..bab81e3 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -342,7 +342,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto cleanup;
}
-remain = vol-target.allocation;
+remain = vol-target.capacity;
if (inputvol) {
int res = virStorageBackendCopyToFD(vol, inputvol,
@@ -401,6 +401,12 @@ createRawFile(int fd, virStorageVolDefPtr vol,
int ret = 0;
unsigned long long pos = 0;
+/* If the new allocation is lower than the capacity of the orignal file,
s/orignal/original
ACK series
John
+ * the cloned volume will be sparse */
+if (inputvol
+vol-target.allocation inputvol-target.capacity)
+need_alloc = false;
+
/* Seek to the final size, so the capacity is available upfront
* for progress reporting */
if (ftruncate(fd, vol-target.capacity) 0) {
@@ -420,7 +426,7 @@ createRawFile(int fd, virStorageVolDefPtr vol,
* to writing zeroes block by block in case fallocate isn't
* available, and since we're going to copy data from another
* file it doesn't make sense to write the file twice. */
-if (vol-target.allocation) {
+if (vol-target.allocation need_alloc) {
if (fallocate(fd, 0, 0, vol-target.allocation) == 0) {
need_alloc = false;
} else if (errno != ENOSYS errno != EOPNOTSUPP) {
@@ -433,21 +439,20 @@ createRawFile(int fd, virStorageVolDefPtr vol,
}
#endif
-
if (inputvol) {
-unsigned long long remain = vol-target.allocation;
+unsigned long long remain = inputvol-target.capacity;
/* allow zero blocks to be skipped if we've requested sparse
* allocation (allocation capacity) or we have already
* been able to allocate the required space. */
-bool want_sparse = !need_alloc ||
-(vol-target.allocation inputvol-target.capacity);
-
ret = virStorageBackendCopyToFD(vol, inputvol, fd, remain,
-want_sparse, reflink_copy);
+!need_alloc, reflink_copy);
if (ret 0)
goto cleanup;
-pos = vol-target.allocation - remain;
+/* If the new allocation is greater than the original capacity,
+ * but fallocate failed, fill the rest with zeroes.
+ */
+pos = inputvol-target.capacity - remain;
}
if (need_alloc) {
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index e600514..ba27acf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1975,11 +1975,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
if (newvol-target.capacity origvol-target.capacity)
newvol-target.capacity = origvol-target.capacity;
-/* Make sure allocation is at least as large as the destination cap,
- * to make absolutely sure we copy all possible contents */
-if (newvol-target.allocation origvol-target.capacity)
-newvol-target.allocation = origvol-target.capacity;
-
if (!backend-buildVolFrom) {
virReportError(VIR_ERR_NO_SUPPORT,
%s, _(storage pool does not support
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] conf: Add getter for network routes
Add virNetworkDefGetRouteByIndex() similarly to
virNetworkDefGetIpByIndex(), but for routes.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/network_conf.c | 26 ++
src/conf/network_conf.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 30 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 31d4463a475b..72006e9822d7 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -803,6 +803,32 @@ virNetworkDefGetIpByIndex(const virNetworkDef *def,
return NULL;
}
+/* return routes[index], or NULL if there aren't enough routes */
+virNetworkRouteDefPtr
+virNetworkDefGetRouteByIndex(const virNetworkDef *def,
+ int family, size_t n)
+{
+size_t i;
+
+if (!def-routes || n = def-nroutes)
+return NULL;
+
+if (family == AF_UNSPEC)
+return def-routes[n];
+
+/* find the nth route of type family */
+for (i = 0; i def-nroutes; i++) {
+virSocketAddrPtr addr = virNetworkRouteDefGetAddress(def-routes[i]);
+if (VIR_SOCKET_ADDR_IS_FAMILY(addr, family)
+ (n-- = 0)) {
+return def-routes[i];
+}
+}
+
+/* failed to find enough of the right family */
+return NULL;
+}
+
/* return number of 1 bits in netmask for the network's ipAddress,
* or -1 on error
*/
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 9411a02e32cb..1cd5100c1278 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -360,6 +360,9 @@ virPortGroupDefPtr virPortGroupFindByName(virNetworkDefPtr
net,
virNetworkIpDefPtr
virNetworkDefGetIpByIndex(const virNetworkDef *def,
int family, size_t n);
+virNetworkRouteDefPtr
+virNetworkDefGetRouteByIndex(const virNetworkDef *def,
+ int family, size_t n);
int virNetworkIpDefPrefix(const virNetworkIpDef *def);
int virNetworkIpDefNetmask(const virNetworkIpDef *def,
virSocketAddrPtr netmask);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1566d11e4156..9d117acbd2bd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -598,6 +598,7 @@ virNetworkDefFormat;
virNetworkDefFormatBuf;
virNetworkDefFree;
virNetworkDefGetIpByIndex;
+virNetworkDefGetRouteByIndex;
virNetworkDefParseFile;
virNetworkDefParseNode;
virNetworkDefParseString;
--
2.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: Remember incoming migration errors
On Wed, Jul 08, 2015 at 15:22:50 +0200, Jiri Denemark wrote:
If QEMU fails during incoming migration, the domain disappears including
a possibly useful error message read from QEMU log file. Let's remember
the error in virQEMUDriver so that Finish can report more than just no
such domain.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_conf.h | 3 +++
src/qemu/qemu_driver.c| 31 +++---
src/qemu/qemu_migration.c | 55
++-
src/qemu/qemu_migration.h | 7 ++
src/qemu/qemu_monitor.c | 19
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_process.c | 4
7 files changed, 112 insertions(+), 9 deletions(-)
...
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a57a177..82069a1 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
...
@@ -6051,3 +6054,53 @@ qemuMigrationJobFinish(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
qemuDomainObjEndAsyncJob(driver, vm);
}
+
+
+static void
+qemuMigrationErrorFree(void *data,
+ const void *name ATTRIBUTE_UNUSED)
+{
+virErrorPtr err = data;
+virFreeError(err);
+}
+
+int
+qemuMigrationErrorInit(virQEMUDriverPtr driver)
+{
+driver-migrationErrors = virHashLockableNew(64, qemuMigrationErrorFree);
+if (driver-migrationErrors)
+return 0;
+else
+return -1;
+}
+
This function consumes @err. A comment noting that would be helpful.
+void
+qemuMigrationErrorSave(virQEMUDriverPtr driver,
+ const char *name,
+ virErrorPtr err)
+{
+if (!err)
+return;
+
+VIR_DEBUG(Saving incoming migration error for domain %s: %s,
+ name, err-message);
+if (virHashLockableUpdate(driver-migrationErrors, name, err) 0) {
+VIR_WARN(Failed to save migration error for domain '%s', name);
+virFreeError(err);
+}
+}
+
+void
+qemuMigrationErrorReport(virQEMUDriverPtr driver,
+ const char *name)
+{
+virErrorPtr err;
+
+if (!(err = virHashLockableSteal(driver-migrationErrors, name)))
+return;
+
+VIR_DEBUG(Restoring saved incoming migration error for domain %s: %s,
+ name, err-message);
+virSetError(err);
+virFreeError(err);
+}
...
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 896d9fd..9db05c5 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1057,6 +1057,25 @@ qemuMonitorSend(qemuMonitorPtr mon,
}
Telling that the user is responsible for freeing the value would be
helpful here.
+virErrorPtr
+qemuMonitorLastError(qemuMonitorPtr mon)
+{
+virErrorPtr old;
+virErrorPtr err;
+
+if (mon-lastError.code == VIR_ERR_OK)
+return NULL;
+
+old = virSaveLastError();
+virSetError(mon-lastError);
+err = virSaveLastError();
+virSetError(old);
+virFreeError(old);
Ummm, how about exporting virCopyError rather than using this rather
opaque and ugly way to copy the error?
+
+return err;
+}
+
+
virJSONValuePtr
qemuMonitorGetOptions(qemuMonitorPtr mon)
{
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] vz: fix cleanup of nets of bridged type
We create a virtual network of special type, which
has the same name as bridge name to create bridged
network adapter in vz. So when we delete such an
adapter we have to remove corresponding virtual
network.
So let's rename prlsdkDelNet to prlsdkCleanupBridgedNet
and don't check for return value.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
src/vz/vz_sdk.c | 25 -
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index a312990..d1bc312 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2986,20 +2986,15 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
return ret;
}
-static int
-prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net)
+static void
+prlsdkCleanupBridgedNet(vzConnPtr privconn, virDomainNetDefPtr net)
{
-int ret = -1;
PRL_RESULT pret;
PRL_HANDLE vnet = PRL_INVALID_HANDLE;
PRL_HANDLE job = PRL_INVALID_HANDLE;
-if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
- _(unplugging network device of type %s is not
supported),
- virDomainNetTypeToString(net-type));
-return ret;
-}
+if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE)
+return;
pret = PrlVirtNet_Create(vnet);
prlsdkCheckRetGoto(pret, cleanup);
@@ -3011,11 +3006,8 @@ prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net)
if (PRL_FAILED(pret = waitJob(job)))
goto cleanup;
-ret = 0;
-
cleanup:
PrlHandle_Free(vnet);
-return ret;
}
int prlsdkAttachNet(virDomainObjPtr dom,
@@ -3107,8 +3099,7 @@ int prlsdkDetachNet(virDomainObjPtr dom,
if (sdknet == PRL_INVALID_HANDLE)
goto cleanup;
-if (prlsdkDelNet(privconn, net) 0)
-goto cleanup;
+prlsdkCleanupBridgedNet(privconn, net);
pret = PrlVmDev_Remove(sdknet);
prlsdkCheckRetGoto(pret, cleanup);
@@ -3530,7 +3521,7 @@ prlsdkDoApplyConfig(virConnectPtr conn,
if (olddef) {
for (i = 0; i olddef-nnets; i++)
-prlsdkDelNet(conn-privateData, olddef-nets[i]);
+prlsdkCleanupBridgedNet(conn-privateData, olddef-nets[i]);
}
for (i = 0; i def-nnets; i++) {
@@ -3575,7 +3566,7 @@ prlsdkDoApplyConfig(virConnectPtr conn,
VIR_FREE(mask);
for (i = 0; i def-nnets; i++)
-prlsdkDelNet(conn-privateData, def-nets[i]);
+prlsdkCleanupBridgedNet(conn-privateData, def-nets[i]);
return -1;
}
@@ -3722,7 +3713,7 @@ prlsdkUnregisterDomain(vzConnPtr privconn,
virDomainObjPtr dom)
size_t i;
for (i = 0; i dom-def-nnets; i++)
-prlsdkDelNet(privconn, dom-def-nets[i]);
+prlsdkCleanupBridgedNet(privconn, dom-def-nets[i]);
job = PrlVm_Unreg(privdom-sdkdom);
if (PRL_FAILED(waitJob(job)))
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: don't use initialized ret in qemuRemoveSharedDevice
This fixes
CC qemu/libvirt_driver_qemu_impl_la-qemu_conf.lo
qemu/qemu_conf.c: In function 'qemuRemoveSharedDevice':
qemu/qemu_conf.c:1384:9: error: 'ret' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
---
This is only catched by Debian Wheezy's gcc 4.7.2, Jessies gcc 4.9.2 doesn't
trap on it.
src/qemu/qemu_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2ab5494..38d4a86 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1381,7 +1381,7 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver,
{
char *dev_path = NULL;
char *key = NULL;
-int ret;
+int ret = -1;
if (!qemuIsSharedHostdev(hostdev))
return 0;
--
2.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/2] Fix nodeinfo output on PPC64 KVM hosts
On 07/07/2015 03:25 AM, Andrea Bolognani wrote: Changes from v3 to v4: * removed a printf() statement; * fixed typo in a commit message. Shivaprasad G Bhat (2): Fix nodeinfo output on PPC64 KVM hosts Add testcase for PPC64 kvm host nodeinfo Never saw the v4 2/2 come through (nor do I see it in the archive); however, I assume it's the same as the v3 patch: http://www.redhat.com/archives/libvir-list/2015-July/msg00155.html Given it is and what I found reviewing the following: http://www.redhat.com/archives/libvir-list/2015-July/msg00219.html regarding nodeinfo.c not really using the tests/nodeinfodata local path instead the running host's sysfs (/sys/devices/system) path. I found while testing that the proposed patch wouldn't run correctly on my host because my /sys/devices/system/cpu/present is 0-3 and the patch would fail on any test with cpu4+ since the tests/nodeinfodata/ present file isn't referenced (if it existed). I created a series which adjusts the SYSFS_SYSTEM_PATH logic in nodeinfo.c to allow for a supplied path or uses the default: http://www.redhat.com/archives/libvir-list/2015-July/msg00278.html Not looking for a review of the 9 patch sysfs series, but I am curious to get a perspective on the patch I initially reviewed which modifies virNodeParseNode to filter out or exclude cpu's that are offline because they're defective/empty and perhaps how/if that applies to this environment as well. I'm also curious what happens if the 2/2 patch is run on a PPC64 host with less than 96 cores (from .../cpu/present) since the results seem to expect the 96 cores to be present. It would seem the existing code without the sysfs path redirection would fail, since the caller linuxNodeInfoCPUPopulate would be using the host's sysfs path rather than the tests sysfs path. John src/libvirt_private.syms | 1 + src/nodeinfo.c | 138 +++-- src/nodeinfo.h | 1 + tests/Makefile.am | 6 + tests/nodeinfodata/linux-ppc64-subcores.cpuinfo| 59 + tests/nodeinfodata/linux-ppc64-subcores.expected | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu0/online | 1 + .../linux-subcores/cpu/cpu0/physical_id| 1 + .../linux-subcores/cpu/cpu0/topology/core_id | 1 + .../linux-subcores/cpu/cpu0/topology/core_siblings | 1 + .../cpu/cpu0/topology/core_siblings_list | 1 + .../cpu/cpu0/topology/physical_package_id | 1 + .../cpu/cpu0/topology/thread_siblings | 1 + .../cpu/cpu0/topology/thread_siblings_list | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu1/online | 1 + .../linux-subcores/cpu/cpu1/physical_id| 1 + tests/nodeinfodata/linux-subcores/cpu/cpu10/online | 1 + .../linux-subcores/cpu/cpu10/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu11/online | 1 + .../linux-subcores/cpu/cpu11/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu12/online | 1 + .../linux-subcores/cpu/cpu12/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu13/online | 1 + .../linux-subcores/cpu/cpu13/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu14/online | 1 + .../linux-subcores/cpu/cpu14/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu15/online | 1 + .../linux-subcores/cpu/cpu15/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu16/online | 1 + .../linux-subcores/cpu/cpu16/physical_id | 1 + .../linux-subcores/cpu/cpu16/topology/core_id | 1 + .../cpu/cpu16/topology/core_siblings | 1 + .../cpu/cpu16/topology/core_siblings_list | 1 + .../cpu/cpu16/topology/physical_package_id | 1 + .../cpu/cpu16/topology/thread_siblings | 1 + .../cpu/cpu16/topology/thread_siblings_list| 1 + tests/nodeinfodata/linux-subcores/cpu/cpu17/online | 1 + .../linux-subcores/cpu/cpu17/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu18/online | 1 + .../linux-subcores/cpu/cpu18/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu19/online | 1 + .../linux-subcores/cpu/cpu19/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu2/online | 1 + .../linux-subcores/cpu/cpu2/physical_id| 1 + tests/nodeinfodata/linux-subcores/cpu/cpu20/online | 1 + .../linux-subcores/cpu/cpu20/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu21/online | 1 + .../linux-subcores/cpu/cpu21/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu22/online | 1 + .../linux-subcores/cpu/cpu22/physical_id | 1 + tests/nodeinfodata/linux-subcores/cpu/cpu23/online | 1 +
[libvirt] [PATCH] qemu: Reject updating unsupported disk information
If one calls update-device with information that is not updatable,
libvirt reports success even though no data were updated. The example
used in the bug linked below uses updating device with boot order='2'/
which, in my opinion, is a valid thing to request from user's
perspective. Mainly since we properly error out if user wants to update
such data on a network device for example.
And since there are many things that might happen (update-device on disk
basically knows just how to change removable media), check for what's
changing and moreover, since the function might be usable in other
dirvers (updating only disk path is a valid possibility) let's abstract
it for any two disks.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1007228
---
src/conf/domain_conf.c | 111 +++
src/conf/domain_conf.h | 2 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 ++
4 files changed, 117 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0219c3c4814d..a6950087d987 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5687,6 +5687,117 @@ virDomainDiskFindByBusAndDst(virDomainDefPtr def,
return NULL;
}
+
+/*
+ * Makes sure the @disk differs from @orig_disk only by the source
+ * path and nothing else. Fields that are being checked and the
+ * information whether they are nullable (can be NULL) or is taken
+ * from the virDomainDiskDefFormat() code.
+ */
+bool
+virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk,
+ virDomainDiskDefPtr orig_disk)
+{
+#define CHECK_EQ(field, field_name, nullable) \
+do {\
+if (nullable !disk-field) \
+break; \
+if (disk-field != orig_disk-field) { \
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \
+ _(cannot modify field '%s' of the disk), \
+ field_name); \
+return false; \
+} \
+} while (0)
+
+CHECK_EQ(device, device, false);
+CHECK_EQ(cachemode, cache, true);
+CHECK_EQ(error_policy, error_policy, true);
+CHECK_EQ(rerror_policy, rerror_policy, true);
+CHECK_EQ(iomode, io, true);
+CHECK_EQ(ioeventfd, ioeventfd, true);
+CHECK_EQ(event_idx, event_idx, true);
+CHECK_EQ(copy_on_read, copy_on_read, true);
+CHECK_EQ(discard, discard, true);
+CHECK_EQ(iothread, iothread, true);
+
+if (disk-geometry.cylinders
+disk-geometry.heads
+disk-geometry.sectors) {
+CHECK_EQ(geometry.cylinders, geometry cylinders, false);
+CHECK_EQ(geometry.heads, geometry heads, false);
+CHECK_EQ(geometry.sectors, geometry sectors, false);
+CHECK_EQ(geometry.trans, BIOS-translation-modus, true);
+}
+
+CHECK_EQ(blockio.logical_block_size,
+ blockio logical_block_size, false);
+CHECK_EQ(blockio.physical_block_size,
+ blockio physical_block_size, false);
+
+if (disk-bus == VIR_DOMAIN_DISK_BUS_USB)
+CHECK_EQ(removable, removable, true);
+
+CHECK_EQ(blkdeviotune.total_bytes_sec,
+ blkdeviotune.total_bytes_sec,
+ true);
+CHECK_EQ(blkdeviotune.read_bytes_sec,
+ blkdeviotune.read_bytes_sec,
+ true);
+CHECK_EQ(blkdeviotune.write_bytes_sec,
+ blkdeviotune.write_bytes_sec,
+ true);
+CHECK_EQ(blkdeviotune.total_iops_sec,
+ blkdeviotune.total_iops_sec,
+ true);
+CHECK_EQ(blkdeviotune.read_iops_sec,
+ blkdeviotune.read_iops_sec,
+ true);
+CHECK_EQ(blkdeviotune.write_iops_sec,
+ blkdeviotune.write_iops_sec,
+ true);
+CHECK_EQ(blkdeviotune.total_bytes_sec_max,
+ blkdeviotune.total_bytes_sec_max,
+ true);
+CHECK_EQ(blkdeviotune.read_bytes_sec_max,
+ blkdeviotune.read_bytes_sec_max,
+ true);
+CHECK_EQ(blkdeviotune.write_bytes_sec_max,
+ blkdeviotune.write_bytes_sec_max,
+ true);
+CHECK_EQ(blkdeviotune.total_iops_sec_max,
+ blkdeviotune.total_iops_sec_max,
+ true);
+CHECK_EQ(blkdeviotune.read_iops_sec_max,
+ blkdeviotune.read_iops_sec_max,
+ true);
+CHECK_EQ(blkdeviotune.write_iops_sec_max,
+ blkdeviotune.write_iops_sec_max,
+ true);
+CHECK_EQ(blkdeviotune.size_iops_sec,
+ blkdeviotune.size_iops_sec,
+ true);
+
+CHECK_EQ(transient, transient, true);
+CHECK_EQ(serial, serial, true);
+CHECK_EQ(wwn, wwn,
Re: [libvirt] [PATCH] vz: fix cleanup of nets of bridged type
09.07.2015 19:20, Dmitry Guryanov пишет:
We create a virtual network of special type, which
has the same name as bridge name to create bridged
network adapter in vz. So when we delete such an
adapter we have to remove corresponding virtual
network.
So let's rename prlsdkDelNet to prlsdkCleanupBridgedNet
and don't check for return value.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
src/vz/vz_sdk.c | 25 -
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index a312990..d1bc312 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2986,20 +2986,15 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
return ret;
}
-static int
-prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net)
+static void
+prlsdkCleanupBridgedNet(vzConnPtr privconn, virDomainNetDefPtr net)
{
-int ret = -1;
PRL_RESULT pret;
PRL_HANDLE vnet = PRL_INVALID_HANDLE;
PRL_HANDLE job = PRL_INVALID_HANDLE;
-if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
- _(unplugging network device of type %s is not
supported),
- virDomainNetTypeToString(net-type));
-return ret;
-}
+if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE)
+return;
pret = PrlVirtNet_Create(vnet);
prlsdkCheckRetGoto(pret, cleanup);
@@ -3011,11 +3006,8 @@ prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net)
if (PRL_FAILED(pret = waitJob(job)))
goto cleanup;
-ret = 0;
-
cleanup:
PrlHandle_Free(vnet);
-return ret;
}
int prlsdkAttachNet(virDomainObjPtr dom,
@@ -3107,8 +3099,7 @@ int prlsdkDetachNet(virDomainObjPtr dom,
if (sdknet == PRL_INVALID_HANDLE)
goto cleanup;
-if (prlsdkDelNet(privconn, net) 0)
-goto cleanup;
+prlsdkCleanupBridgedNet(privconn, net);
pret = PrlVmDev_Remove(sdknet);
prlsdkCheckRetGoto(pret, cleanup);
@@ -3530,7 +3521,7 @@ prlsdkDoApplyConfig(virConnectPtr conn,
if (olddef) {
for (i = 0; i olddef-nnets; i++)
-prlsdkDelNet(conn-privateData, olddef-nets[i]);
+prlsdkCleanupBridgedNet(conn-privateData, olddef-nets[i]);
}
for (i = 0; i def-nnets; i++) {
@@ -3575,7 +3566,7 @@ prlsdkDoApplyConfig(virConnectPtr conn,
VIR_FREE(mask);
for (i = 0; i def-nnets; i++)
-prlsdkDelNet(conn-privateData, def-nets[i]);
+prlsdkCleanupBridgedNet(conn-privateData, def-nets[i]);
return -1;
}
@@ -3722,7 +3713,7 @@ prlsdkUnregisterDomain(vzConnPtr privconn,
virDomainObjPtr dom)
size_t i;
for (i = 0; i dom-def-nnets; i++)
-prlsdkDelNet(privconn, dom-def-nets[i]);
+prlsdkCleanupBridgedNet(privconn, dom-def-nets[i]);
job = PrlVm_Unreg(privdom-sdkdom);
if (PRL_FAILED(waitJob(job)))
ACK. Looks better
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Reject updating unsupported disk information
On Thu, Jul 09, 2015 at 18:36:00 +0200, Martin Kletzander wrote:
If one calls update-device with information that is not updatable,
libvirt reports success even though no data were updated. The example
used in the bug linked below uses updating device with boot order='2'/
which, in my opinion, is a valid thing to request from user's
perspective. Mainly since we properly error out if user wants to update
such data on a network device for example.
And since there are many things that might happen (update-device on disk
basically knows just how to change removable media), check for what's
changing and moreover, since the function might be usable in other
dirvers (updating only disk path is a valid possibility) let's abstract
it for any two disks.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1007228
---
src/conf/domain_conf.c | 111
+++
src/conf/domain_conf.h | 2 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 ++
4 files changed, 117 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0219c3c4814d..a6950087d987 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5687,6 +5687,117 @@ virDomainDiskFindByBusAndDst(virDomainDefPtr def,
return NULL;
}
+
+/*
+ * Makes sure the @disk differs from @orig_disk only by the source
+ * path and nothing else. Fields that are being checked and the
+ * information whether they are nullable (can be NULL) or is taken
Um, NULL? see below ...
+ * from the virDomainDiskDefFormat() code.
+ */
+bool
+virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk,
+ virDomainDiskDefPtr orig_disk)
+{
+#define CHECK_EQ(field, field_name, nullable) \
+do {\
+if (nullable !disk-field) \
+break; \
+if (disk-field != orig_disk-field) { \
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \
+ _(cannot modify field '%s' of the disk), \
+ field_name); \
+return false; \
+} \
+} while (0)
+
+CHECK_EQ(device, device, false);
+CHECK_EQ(cachemode, cache, true);
Lets take the line below as an example.
+CHECK_EQ(error_policy, error_policy, true);
If disk-error_policy == VIR_DOMAIN_DISK_ERROR_POLICY_DEFAULT (which
equals to 0) the check will be skipped and thus the error_policy setting
might be different and we would not report an error.
+CHECK_EQ(rerror_policy, rerror_policy, true);
+CHECK_EQ(iomode, io, true);
+CHECK_EQ(ioeventfd, ioeventfd, true);
+CHECK_EQ(event_idx, event_idx, true);
+CHECK_EQ(copy_on_read, copy_on_read, true);
+CHECK_EQ(discard, discard, true);
+CHECK_EQ(iothread, iothread, true);
+
+if (disk-geometry.cylinders
+disk-geometry.heads
+disk-geometry.sectors) {
+CHECK_EQ(geometry.cylinders, geometry cylinders, false);
+CHECK_EQ(geometry.heads, geometry heads, false);
+CHECK_EQ(geometry.sectors, geometry sectors, false);
+CHECK_EQ(geometry.trans, BIOS-translation-modus, true);
+}
+
+CHECK_EQ(blockio.logical_block_size,
+ blockio logical_block_size, false);
+CHECK_EQ(blockio.physical_block_size,
+ blockio physical_block_size, false);
+
+if (disk-bus == VIR_DOMAIN_DISK_BUS_USB)
+CHECK_EQ(removable, removable, true);
+
As an example of the field below. The field is an integer ...
+CHECK_EQ(blkdeviotune.total_bytes_sec,
+ blkdeviotune.total_bytes_sec,
+ true);
+CHECK_EQ(blkdeviotune.read_bytes_sec,
+ blkdeviotune.read_bytes_sec,
+ true);
+CHECK_EQ(blkdeviotune.write_bytes_sec,
+ blkdeviotune.write_bytes_sec,
+ true);
+CHECK_EQ(blkdeviotune.total_iops_sec,
+ blkdeviotune.total_iops_sec,
+ true);
+CHECK_EQ(blkdeviotune.read_iops_sec,
+ blkdeviotune.read_iops_sec,
+ true);
+CHECK_EQ(blkdeviotune.write_iops_sec,
+ blkdeviotune.write_iops_sec,
+ true);
+CHECK_EQ(blkdeviotune.total_bytes_sec_max,
+ blkdeviotune.total_bytes_sec_max,
+ true);
+CHECK_EQ(blkdeviotune.read_bytes_sec_max,
+ blkdeviotune.read_bytes_sec_max,
All of the usage of CHECK_EQ use the same text in the error message as
is the field name. You could perhaps use the strigification macro
operator.
+ true);
+
Re: [libvirt] [PATCH] qemu: don't use initialized ret in qemuRemoveSharedDevice
On Thu, Jul 09, 2015 at 19:22:30 +0200, Guido Günther wrote: This fixes CC qemu/libvirt_driver_qemu_impl_la-qemu_conf.lo qemu/qemu_conf.c: In function 'qemuRemoveSharedDevice': qemu/qemu_conf.c:1384:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] --- This is only catched by Debian Wheezy's gcc 4.7.2, Jessies gcc 4.9.2 doesn't trap on it. src/qemu/qemu_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Oops. I've missed that in my review. ACK if you didn't push this already. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't use initialized ret in qemuRemoveSharedDevice
Hi, On Thu, Jul 09, 2015 at 07:28:34PM +0200, Peter Krempa wrote: On Thu, Jul 09, 2015 at 19:22:30 +0200, Guido Günther wrote: This fixes CC qemu/libvirt_driver_qemu_impl_la-qemu_conf.lo qemu/qemu_conf.c: In function 'qemuRemoveSharedDevice': qemu/qemu_conf.c:1384:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] --- This is only catched by Debian Wheezy's gcc 4.7.2, Jessies gcc 4.9.2 doesn't trap on it. src/qemu/qemu_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Oops. I've missed that in my review. ACK if you didn't push this already. Thanks for the quick response! I would have pushed this under the build breaker rule but since I'm a bit disconnected with the ML at the moment I thought I'd rather post a patch first. Pushed now. Thanks! -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Check duplicate WWNs also for hotplugged disks
In commit 714b38cb232bcbbd7487af4c058fa6d0999b3326 I tried to avoid
having two disks with the same WWN in a VM. I forgot to check the
hotplug paths though which make it possible bypass that check. Reinforce
the fix by checking the wwn when attaching the disk.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1208009
---
src/conf/domain_conf.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1f7862b..5a9a88d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22135,6 +22135,34 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def
ATTRIBUTE_UNUSED,
return 0;
}
+
+/**
+ * virDomainDefGetDiskByWWN:
+ * @def: domain definition
+ * @wwn: wwn of a disk to find
+ *
+ * Returns a disk definition pointer corresponding to the given WWN identifier
+ * or NULL either if @wwn was NULL or if disk with given WWN is not present in
+ * the domain definition.
+ */
+static virDomainDiskDefPtr
+virDomainDefGetDiskByWWN(virDomainDefPtr def,
+ const char *wwn)
+{
+size_t i;
+
+if (!wwn)
+return NULL;
+
+for (i = 0; i def-ndisks; i++) {
+if (STREQ_NULLABLE(def-disks[i]-wwn, wwn))
+return def-disks[i];
+}
+
+return NULL;
+}
+
+
int
virDomainDefCompatibleDevice(virDomainDefPtr def,
virDomainDeviceDefPtr dev,
@@ -22178,6 +22206,15 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
}
}
+if (dev-type == VIR_DOMAIN_DEVICE_DISK) {
+if (!!virDomainDefGetDiskByWWN(def, dev-data.disk-wwn)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Domain already has a disk with wwn '%s'),
+ dev-data.disk-wwn);
+return -1;
+}
+}
+
return 0;
}
--
2.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [java] [PATCH 0/6] Fix JNA wrapping, fix memory leaks and wrap security model / label function
At Wed, 08 Jul 2015 14:42:52 +0200, Michal Privoznik wrote: Claudio Bley (6): JNA: fix wrong return type void vs. int JNA: add CString class and fix memory leaks JNA: simplify freeing memory for C strings Use the CString class for Arrays of CStrings too Implement Domain.getSecurityLabel and add SecurityLabel class Implement Connect.getSecurityModel and add SecurityModel class It has been a while since I posted these patches. Because nobody objected until now, I'm just going to push them. Yes, that's the same approach I'm using for libvirt-php, where the reviewer's capacity consists of, well, just me. I guess it's okay until the time those projects drag more attention. Thanks, I'll do the same then. Took me a while, but pushed now (after all this time I had some _difficulties_ remembering the passphrase of my SSH key... ;-)) -- Claudio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.2.17
On Thu, Jul 09, 2015 at 02:00:10PM -0700, Peter Kieser wrote: On 2015-06-30 2:49 AM, Daniel Veillard wrote: On Tue, Jun 30, 2015 at 11:00:24AM +0200, Guido Günther wrote: On Tue, Jun 30, 2015 at 03:00:09PM +0800, Daniel Veillard wrote: On Mon, Jun 29, 2015 at 10:34:55PM +0200, Guido Günther wrote: On Sun, Jun 28, 2015 at 01:00:01PM +0800, Daniel Veillard wrote: Following discussions on Friday, I applied the patches to deactivate the subset of Admin APIs and revert from 1.3.0 to 1.2.17. I then tagged in git and pushed signed tarballs and rpms to the usual place: ftp://libvirt.org/pub/libvirt/ I didn't run my usual tests on that one, my infra is in flux, so even more reasons for people to give it a try :-) I'm likely to make a candidate release 2 on Tuesday and if all goes well we can push 1.2.17 on Thursday, Building the tarball fails for me with: make[4]: Entering directory '/tmp/buildd/libvirt-1.2.17~rc1/debian/build/docs' missing XHTML1 DTD cat: internals/locking.html.tmp: No such file or directory Makefile:2385: recipe for target 'internals/locking.html' failed make[4]: *** [internals/locking.html] Error 1 The missing XHTML1 DTD just means you can't validate the locking.html.tmp against a local copy of the DTD for XHTML1, but then it seems that the locking.html.tmp wasn't generated. It should be generated via xsltproc, it seems it's missing in your build environment, make sure you have it. Make sure you have xmllint, xsltproc and xhtml1-dtds in your build system, I should have added that I tried this with and without xsltproc + xmllint. I now also added the DTDs but no change (the build env didn't change since the last release). humpf ... locking.html.in wasn't touched since Feb, that need more attention and building from the tarballs worked here, strange Daniel http://libvirt.org/git/?p=libvirt.git;a=commit;h=1310b1358cdf9c8acba6e0e85feb869241e59faa I had to revert this commit to get 1.2.17 to build under debian packaging chroot. Weird, I don't see why the .html.tmp .html rules fails to apply then ... it seems to work for everything else Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 0/4] gconfig: Leak fixes and small cleanup
Hey, This patch series makes tests/test-gconfig valgrind-clean, and refactors two setters in GVirConfigDomainVideo to make them use the helpers provided by GVirConfigObject. Christophe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5] qemu: Enable migration events on QMP monitor
On Thu, Jul 09, 2015 at 09:11:16 +0200, Jiri Denemark wrote: Even if QEMU supports migration events it doesn't send them by default. We have to enable them by calling migrate-set-capabilities. Let's enable migration events everytime we can and clear QEMU_CAPS_MIGRATION_EVENT in case migrate-set-capabilities does not support events. Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 5: - simplified Version 4: - new patch src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_process.c | 36 3 files changed, 26 insertions(+), 13 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 3/4] test-gconfig: Test video heads/vram setting
--- tests/test-gconfig.c | 2 ++ tests/xml/gconfig-domain-device-video.xml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c index de09c24..aca8f02 100644 --- a/tests/test-gconfig.c +++ b/tests/test-gconfig.c @@ -494,6 +494,8 @@ static void test_domain_device_video(void) video = gvir_config_domain_video_new(); gvir_config_domain_video_set_model(video, GVIR_CONFIG_DOMAIN_VIDEO_MODEL_QXL); +gvir_config_domain_video_set_heads(video, 4); +gvir_config_domain_video_set_vram(video, 256*1024); gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(video)); g_object_unref(G_OBJECT(video)); diff --git a/tests/xml/gconfig-domain-device-video.xml b/tests/xml/gconfig-domain-device-video.xml index a6155e2..7af6256 100644 --- a/tests/xml/gconfig-domain-device-video.xml +++ b/tests/xml/gconfig-domain-device-video.xml @@ -1,7 +1,7 @@ domain devices video - model type=qxl/ + model type=qxl heads=4 vram=262144/ /video /devices /domain -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 1/4] gconfig: Fix leak in gvir_config_domain_filesys_set_ram_usage
The object returned by gvir_config_object_replace_child() must be unref'ed when no longer needed. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 860480c..97b7bd6 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c @@ -201,7 +201,7 @@ void gvir_config_domain_filesys_set_ram_usage(GVirConfigDomainFilesys *filesys, usage, G_TYPE_UINT64, bytes, units, G_TYPE_STRING, bytes, NULL); - +g_object_unref(G_OBJECT(src)); } void gvir_config_domain_filesys_set_target(GVirConfigDomainFilesys *filesys, -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Don't allow duplicated targets regardless of bus
On Thu, Jun 18, 2015 at 04:12:10PM -0400, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1142631 Commit id 'e0e290552' added a check to determine if the same bus had the same target value. It seems that's not quite good enough as the check should check the target name value regardless of bus type. Also added a DO_TEST_DIFFERENT to exhibit the issue Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 3 +- .../qemuxml2argv-disk-same-targets.xml | 35 ++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-same-targets.xml ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] shall libvirtd validate guest's name ?
On Thu, Jul 09, 2015 at 05:19:29PM +0800, zhang bo wrote: linux-ZyvZnF:~ # virsh list --all IdName State - redhat7;reboot shut off - oscar-vm-5 shut off As shown above, 1 we use command virsh define a.xml to define a guest with a name containing ';', that's 'redhat7;reboot' 2 then we start the guest: virsh start redhat7;reboot 3 shell consider the command as a) run virsh start redhat7, failed b) run reboot, to reboot the host And *the host get rebooted*. shall libvirtd do the guest-name-validation work? Or other suggustions? Semicolon is a strange but valid character to use in a domain's name. It's the user's responsibility to escape any strange characters in the shell prompt. We don't allow '/' in domain names because we use them in filenames. Explicitly rejecting it would be nicer than letting the file creation fail, but rejecting any new characters would unnecessarily break existing domains. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] shall libvirtd validate guest's name ?
On Thu, 2015-07-09 at 17:19 +0800, zhang bo wrote: linux-ZyvZnF:~ # virsh list --all IdName State - redhat7;reboot shut off - oscar-vm-5 shut off As shown above, 1 we use command virsh define a.xml to define a guest with a name containing ';', that's 'redhat7;reboot' 2 then we start the guest: virsh start redhat7;reboot 3 shell consider the command as a) run virsh start redhat7, failed b) run reboot, to reboot the host And *the host get rebooted*. shall libvirtd do the guest-name-validation work? Or other suggustions? Proper usage of string escaping is the user's responsibility. Unfortunately a lot of shell scripts get this wrong, leading to more or less catastrophic results. The same reasoning, by the way, applies to file names, and eg. the Nautilus file manager happily allows me to use foo;reboot as name when creating or renaming files... Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5] qemu: Enable migration events on QMP monitor
Even if QEMU supports migration events it doesn't send them by default.
We have to enable them by calling migrate-set-capabilities. Let's enable
migration events everytime we can and clear QEMU_CAPS_MIGRATION_EVENT in
case migrate-set-capabilities does not support events.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
Notes:
Version 5:
- simplified
Version 4:
- new patch
src/qemu/qemu_monitor.c | 2 +-
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_process.c | 36
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fb325b6..54695c2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -163,7 +163,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
VIR_ENUM_IMPL(qemuMonitorMigrationCaps,
QEMU_MONITOR_MIGRATION_CAPS_LAST,
- xbzrle, auto-converge, rdma-pin-all)
+ xbzrle, auto-converge, rdma-pin-all, events)
VIR_ENUM_IMPL(qemuMonitorVMStatus,
QEMU_MONITOR_VM_STATUS_LAST,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8555f7b..ab7d5a7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -512,6 +512,7 @@ typedef enum {
QEMU_MONITOR_MIGRATION_CAPS_XBZRLE,
QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE,
QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL,
+QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
QEMU_MONITOR_MIGRATION_CAPS_LAST
} qemuMonitorMigrationCaps;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ba84182..1d223d3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1546,7 +1546,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver,
virDomainObjPtr vm, int asyncJob,
vm-def) 0) {
VIR_ERROR(_(Failed to set security context for monitor for %s),
vm-def-name);
-goto error;
+return -1;
}
/* Hold an extra reference because we can't allow 'vm' to be
@@ -1578,26 +1578,38 @@ qemuConnectMonitor(virQEMUDriverPtr driver,
virDomainObjPtr vm, int asyncJob,
if (virSecurityManagerClearSocketLabel(driver-securityManager, vm-def)
0) {
VIR_ERROR(_(Failed to clear security context for monitor for %s),
vm-def-name);
-goto error;
+return -1;
}
if (priv-mon == NULL) {
VIR_INFO(Failed to connect monitor for %s, vm-def-name);
-goto error;
+return -1;
}
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
-goto error;
-ret = qemuMonitorSetCapabilities(priv-mon);
-if (ret == 0
-virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON))
-ret = virQEMUCapsProbeQMP(priv-qemuCaps, priv-mon);
+return -1;
+
+if (qemuMonitorSetCapabilities(priv-mon) 0)
+goto cleanup;
+
+if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)
+virQEMUCapsProbeQMP(priv-qemuCaps, priv-mon) 0)
+goto cleanup;
+
+if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATION_EVENT)
+qemuMonitorSetMigrationCapability(priv-mon,
+ QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
+ true) 0) {
+VIR_DEBUG(Cannot enable migration events; clearing capability);
+virQEMUCapsClear(priv-qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
+}
+
+ret = 0;
+
+ cleanup:
if (qemuDomainObjExitMonitor(driver, vm) 0)
-return -1;
-
- error:
-
+ret = -1;
return ret;
}
--
2.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] qemu: Inline qemuGetHostdevPath
On Wed, Jul 08, 2015 at 15:30:26 -0400, John Ferlan wrote: Since a future patch will need the device path generated when adding a shared host device, remove the qemuAddSharedHostdev and inline the two calls into qemuAddSharedHostdev and qemuRemoveSharedHostdev Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_conf.c | 39 --- 1 file changed, 16 insertions(+), 23 deletions(-) ACK signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] conf: Add helpers to insert/remove/find shmem devices in domain def
On Thu, Jul 09, 2015 at 11:44:30AM +0800, lhuang wrote:
On 07/08/2015 08:14 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:
The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 61
src/conf/domain_conf.h | 7 ++
src/libvirt_private.syms | 3 +++
3 files changed, 71 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 306b718..8a8e4f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def,
}
+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+return VIR_APPEND_ELEMENT(def-shmems, def-nshmems, shmem);
+}
+
+
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+size_t i;
+
+for (i = 0; i def-nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def-shmems[i];
+
+ if (STRNEQ_NULLABLE(shmem-name, tmpshmem-name))
+ continue;
+
I think that you shouldn't be able to have two shmem/ elements in
the same domain, and since the name is mandatory, STREQ() should be
enough to check whether you need to return current 'i'.
Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one
guest could have more than one shmem/ element, maybe one is
non-server shmem and another is server shmem, it depends on how to use
it.
Maybe this function could have a bool parameter (something like a
'full_match') that would say whether everything must match or the name
is enough. And it the bool is false, streq is enough, but if it's
true, you could abstract the internals of this for body into
virDomainShmemEquals() or something and that would be called instead.
Well, depending on whether you are unplugging it (you want everything
that was specified to match) or plugging it in (if there's the same
name, just reject it).
Okay, we could just check the name if use the same shmem (both server
and non-server) in the same guest is a invalid case.
Thanks a lot for your review.
Luyao
+ if (shmem-size != tmpshmem-size)
+ continue;
+
+ if (shmem-server.enabled != tmpshmem-server.enabled ||
+ (shmem-server.enabled
+ STRNEQ_NULLABLE(shmem-server.chr.data.nix.path,
+ tmpshmem-server.chr.data.nix.path)))
+ continue;
+
+ if (shmem-msi.enabled != tmpshmem-msi.enabled ||
+ (shmem-msi.enabled
+ (shmem-msi.vectors != tmpshmem-msi.vectors ||
+ shmem-msi.ioeventfd != tmpshmem-msi.ioeventfd)))
+ continue;
+
+if (shmem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
+ !virDomainDeviceInfoAddressIsEqual(shmem-info, tmpshmem-info))
+continue;
+
+break;
+}
+
+if (i def-nshmems)
+return i;
+
+return -1;
+}
+
+
+virDomainShmemDefPtr
+virDomainShmemRemove(virDomainDefPtr def,
+ size_t idx)
+{
+virDomainShmemDefPtr ret = def-shmems[idx];
+
+VIR_DELETE_ELEMENT(def-shmems, idx, def-nshmems);
+
+return ret;
+}
+
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a4b1bf3..39bc928 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2943,6 +2943,13 @@ int
virDomainMemoryFindInactiveByDef(virDomainDefPtr def,
virDomainMemoryDefPtr mem)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virDomainShmemInsert(virDomainDefPtr def,
virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ssize_t virDomainShmemFind(virDomainDefPtr def,
virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def,
size_t idx)
+ATTRIBUTE_NONNULL(1);
+
VIR_ENUM_DECL(virDomainTaint)
VIR_ENUM_DECL(virDomainVirt)
VIR_ENUM_DECL(virDomainBoot)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3ceb4e3..6127f51 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -439,6 +439,9 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemFind;
+virDomainShmemInsert;
+virDomainShmemRemove;
virDomainShutdownReasonTypeFromString;
virDomainShutdownReasonTypeToString;
virDomainShutoffReasonTypeFromString;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix no error settings if fail to find a disk match path
On Thu, Jul 09, 2015 at 11:49:15AM +0800, Luyao Huang wrote:
When we use get blockjob info to a unexist disk path, we will
get a error like this:
# virsh blockjob r7 vdc
error: An error occurred, but the cause is unknown
This is because we do not set the error when jump to endjob.
As virDomainDiskByName won't set the error, we need set them
in the callers function.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 900740e..f134248 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
if (qemuDomainSupportsBlockJobs(vm, NULL) 0)
goto endjob;
-if (!(disk = virDomainDiskByName(vm-def, path, true)))
+if (!(disk = virDomainDiskByName(vm-def, path, true))) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(invalid path %s not assigned to domain), path);
Since the 'path' parameter can be both path and a disk name
(e.g. 'vdc' in your example), I wouldn't use path here because it
looks weird when you get an error saying:
invalid path vdc not assigned to domain
I'd change it to something more abstract like:
Disk '%s' not found in the domain
or anything else but with such specification. If you're OK with that
I'll push it with the modification (also fixing the commit message).
goto endjob;
+}
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm),
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
On Jul 7, 2015 15:51, Ren, Qiaowei wrote: On Jul 6, 2015 14:49, Prerna wrote: On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com mailto:qiaowei@intel.com wrote: One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed. Hi Ren, One query wrt this implementation. I see you make a perf ioctl to gather CMT stats each time the stats API is invoked. If the CMT stats are exposed by a hardware counter, then this implies logging on a per-cpu (or per-socket ???) basis. This also implies that the value read will vary as the CPU (or socket) on which it is being called changes. Now, with this background, if we need real-world stats on a VM, we need this perf ioctl executed on all CPUs/ sockets on which the VM ran. Also, once done, we will need to aggregate results from each of these sources. In this implementation, I am missing this -- there seems no control over which physical CPU the libvirt worker thread will run and collect the perf data from. Data collected from this implementation might not accurately model the system state. I _think_ libvirt currently has no way of directing a worker thread to collect stats from a given CPU -- if we do, I would be happy to learn about it :) Prerna, thanks for your reply. I checked the CMT implementation in kernel, and noticed that the series implement new -count() of pmu driver which can aggregate the results from each cpu if perf type is PERF_TYPE_INTEL_CQM . The following is the link for the patch: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?i d=bfe1fc d2688f557a6b6a88f59ea7619228728bd7 So I guess that this patch just need to set right perf type and cpu=-1. Do you think this is ok? Hi Prerna, Do you have more comments on this patch series? I would be glad to update my implementation. ^-^ Qiaowei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/5] qemu: Enable migration events on QMP monitor
On Wed, Jul 08, 2015 at 18:39:38 +0200, Jiri Denemark wrote:
Even if QEMU supports migration events it doesn't send them by default.
We have to enable them by calling migrate-set-capabilities. Let's enable
migration events everytime we can and clear QEMU_CAPS_MIGRATION_EVENT in
case migrate-set-capabilities does not support events.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
Notes:
Version 4:
- new patch
src/qemu/qemu_monitor.c | 2 +-
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_process.c | 46 ++
3 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fb325b6..54695c2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -163,7 +163,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
VIR_ENUM_IMPL(qemuMonitorMigrationCaps,
QEMU_MONITOR_MIGRATION_CAPS_LAST,
- xbzrle, auto-converge, rdma-pin-all)
+ xbzrle, auto-converge, rdma-pin-all, events)
VIR_ENUM_IMPL(qemuMonitorVMStatus,
QEMU_MONITOR_VM_STATUS_LAST,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8555f7b..ab7d5a7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -512,6 +512,7 @@ typedef enum {
QEMU_MONITOR_MIGRATION_CAPS_XBZRLE,
QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE,
QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL,
+QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
QEMU_MONITOR_MIGRATION_CAPS_LAST
} qemuMonitorMigrationCaps;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ba84182..04e7e93 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1546,7 +1546,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver,
virDomainObjPtr vm, int asyncJob,
vm-def) 0) {
VIR_ERROR(_(Failed to set security context for monitor for %s),
vm-def-name);
-goto error;
+return -1;
}
/* Hold an extra reference because we can't allow 'vm' to be
@@ -1578,26 +1578,48 @@ qemuConnectMonitor(virQEMUDriverPtr driver,
virDomainObjPtr vm, int asyncJob,
if (virSecurityManagerClearSocketLabel(driver-securityManager, vm-def)
0) {
VIR_ERROR(_(Failed to clear security context for monitor for %s),
vm-def-name);
-goto error;
+return -1;
}
if (priv-mon == NULL) {
VIR_INFO(Failed to connect monitor for %s, vm-def-name);
-goto error;
+return -1;
}
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
-goto error;
-ret = qemuMonitorSetCapabilities(priv-mon);
-if (ret == 0
-virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON))
-ret = virQEMUCapsProbeQMP(priv-qemuCaps, priv-mon);
+return -1;
+
+if (qemuMonitorSetCapabilities(priv-mon) 0)
+goto cleanup;
+
+if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)
+virQEMUCapsProbeQMP(priv-qemuCaps, priv-mon) 0)
+goto cleanup;
+
+if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) {
+int rv;
+
+rv = qemuMonitorGetMigrationCapability(
+priv-mon, QEMU_MONITOR_MIGRATION_CAPS_EVENTS);
Is it actually worth querying for the capability? Wouldn't it be
sufficient to just set it and if it fails, clear the capability bit
right away?
+if (rv 0) {
+goto cleanup;
+} else if (rv == 0) {
+VIR_DEBUG(Cannot enable migration events; clearing capability);
+virQEMUCapsClear(priv-qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
+} else if (qemuMonitorSetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
+true) 0) {
Hmm, this formatting looks rather ugly to me. Do we still need to do
this silly 80 column limit in the age of wide angle displays?
+goto cleanup;
+}
+}
+
+ret = 0;
+
+ cleanup:
if (qemuDomainObjExitMonitor(driver, vm) 0)
-return -1;
-
- error:
-
+ret = -1;
return ret;
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib PATCHv5 7/7] gobject: Add wrapper for virNetworkGetDHCPLeases
On Wed, Jul 08, 2015 at 07:00:36PM +0100, Zeeshan Ali (Khattak) wrote: On Wed, Jul 8, 2015 at 3:42 PM, Christophe Fergeau cferg...@redhat.com wrote: but you haven't brought anything forward to support that ugly hack statement in this specific case, #ifdef based solution is going to be ugly, surely you know that. I never made any claims about the magnitude of ugliness, I always want to avoid ugly hacks whenever possible. If it just requires 2 or 3 #ifdef, adding them and forgetting they existed would have been faster than this thread ;) nor any hard data regarding which distros could be impacted by a req bump. I'll stop this discussion until you bring some concrete datapoints to the table. Fair enough! The main point of this discussion was not to convince you but rather to get a third opinion. Honestly, this is a very weird attitude, rather than trying to come with hard facts, you prefer having some kind of poll and make an arbitary uninformed decision. Christophe pgprNruIq4_hU.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/4] qemu: Refactor qemuSetUnprivSGIO return values
On Wed, Jul 08, 2015 at 15:30:27 -0400, John Ferlan wrote: Set to ret = -1 and prove otherwise, like usual Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_conf.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) ACK signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/4] qemu: Fix integer/boolean logic in qemuSetUnprivSGIO
On Wed, Jul 08, 2015 at 15:30:28 -0400, John Ferlan wrote:
Setting of 'val' is a boolean expression, so handle it that way and
adjust the check/return logic to be clearer
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/qemu/qemu_conf.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ddaf7f8..2ab5494 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1433,7 +1433,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
virDomainHostdevDefPtr hostdev = NULL;
char *sysfs_path = NULL;
const char *path = NULL;
-int val = -1;
+bool val;
int ret = -1;
/* sgio is only valid for block disk; cdrom
@@ -1475,8 +1475,12 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
* whitelist is enabled. But if requesting unfiltered access, always
call
* virSetDeviceUnprivSGIO, to report an error for unsupported
unpriv_sgio.
*/
-if ((virFileExists(sysfs_path) || val == 1)
-virSetDeviceUnprivSGIO(path, NULL, val) 0)
+if (!val || !virFileExists(sysfs_path)) {
With this control flow the @val variable can be entirely avoided since
it's just set once and read once.
+ret = 0;
+goto cleanup;
+}
+
+if (virSetDeviceUnprivSGIO(path, NULL, 1) 0)
goto cleanup;
ret = 0;
ACK regardles if you choose to optimize @val out.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix no error settings if fail to find a disk match path
On 07/09/2015 11:49 AM, Luyao Huang wrote:
When we use get blockjob info to a unexist disk path, we will
get a error like this:
# virsh blockjob r7 vdc
error: An error occurred, but the cause is unknown
This is because we do not set the error when jump to endjob.
As virDomainDiskByName won't set the error, we need set them
in the callers function.
Sorry for forget the bz:
https://bugzilla.redhat.com/show_bug.cgi?id=1241355
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 900740e..f134248 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
if (qemuDomainSupportsBlockJobs(vm, NULL) 0)
goto endjob;
-if (!(disk = virDomainDiskByName(vm-def, path, true)))
+if (!(disk = virDomainDiskByName(vm-def, path, true))) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(invalid path %s not assigned to domain), path);
goto endjob;
+}
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm),
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix no error settings if fail to find a disk match path
On 07/09/2015 02:37 PM, Martin Kletzander wrote:
On Thu, Jul 09, 2015 at 11:49:15AM +0800, Luyao Huang wrote:
When we use get blockjob info to a unexist disk path, we will
get a error like this:
# virsh blockjob r7 vdc
error: An error occurred, but the cause is unknown
This is because we do not set the error when jump to endjob.
As virDomainDiskByName won't set the error, we need set them
in the callers function.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 900740e..f134248 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
if (qemuDomainSupportsBlockJobs(vm, NULL) 0)
goto endjob;
-if (!(disk = virDomainDiskByName(vm-def, path, true)))
+if (!(disk = virDomainDiskByName(vm-def, path, true))) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(invalid path %s not assigned to domain),
path);
Since the 'path' parameter can be both path and a disk name
(e.g. 'vdc' in your example), I wouldn't use path here because it
looks weird when you get an error saying:
invalid path vdc not assigned to domain
I'd change it to something more abstract like:
Disk '%s' not found in the domain
or anything else but with such specification. If you're OK with that
I'll push it with the modification (also fixing the commit message).
Okay, i think it will be ok for me as it sounds more friendly.
Thanks a lot for your quick review and help.
Luyao
goto endjob;
+}
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm),
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/3] Allow PCI virtio on ARM virt machine
Virt machine in qemu since v2.3.0 has PCI generic host controller, and can use PCI devices. This provides performance improvement as well as vhost-net with irqfd support for virtio-net. However libvirt currently does not allow ARM virt machine to have PCI devices. This patchset adds the necessary support. Changes since v3: - Capability is based not on qemu version but on support of gpex-pcihost device by qemu - Added a workaround, allowing to pass make check. The problem is that test suite does not build capabilities cache. Unfortunately this means that correct unit-test for the new functionality currently cannot be written. Test suite framework needs to be improved. Changes since v2: Complete rework, use different approach - Correctly model PCI Express bus on the machine. It is now possible to explicitly specify address-type='pci' with attributes. This allows to attach not only virtio, but any other PCI device to the model. - Default is not changed and still mmio, for backwards compatibility with existing installations. PCI bus has to be explicitly specified. - Check for the capability in correct place, in v2 it actually did not work Changes since v1: - Added capability based on qemu version number - Recognize also virt- prefix Pavel Fedin (3): Introduce QEMU_CAPS_OBJECT_GPEX Add PCI-Express root to ARM virt machine Build correct command line for PCI NICs on ARM src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 3 ++- src/qemu/qemu_domain.c | 17 + 4 files changed, 18 insertions(+), 5 deletions(-) -- 1.9.5.msysgit.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/3] Build correct command line for PCI NICs on ARM
Legacy -net option works correctly only with embedded device models, which do not require any bus specification. Therefore, we should use -device for PCI hardware Signed-off-by: Pavel Fedin p.fe...@samsung.com --- src/qemu/qemu_command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b7b85ab..9d6be9f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -457,7 +457,8 @@ qemuDomainSupportsNicdev(virDomainDefPtr def, /* non-virtio ARM nics require legacy -net nic */ if (((def-os.arch == VIR_ARCH_ARMV7L) || (def-os.arch == VIR_ARCH_AARCH64)) -net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) +net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO +net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return false; return true; -- 1.9.5.msysgit.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/3] Add PCI-Express root to ARM virt machine
Here we assume that if qemu supports generic PCI host controller,
it is a part of virt machine and can be used for adding PCI devices.
Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
src/qemu/qemu_domain.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b050a0..c7d14e4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = {
static int
qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
bool addDefaultUSB = true;
bool addImplicitSATA = false;
@@ -1030,12 +1030,21 @@ qemuDomainDefPostParse(virDomainDefPtr def,
break;
case VIR_ARCH_ARMV7L:
- addDefaultUSB = false;
- addDefaultMemballoon = false;
- break;
case VIR_ARCH_AARCH64:
addDefaultUSB = false;
addDefaultMemballoon = false;
+ if (STREQ(def-os.machine, virt) ||
+ STRPREFIX(def-os.machine, virt-)) {
+ virQEMUDriverPtr driver = opaque;
+
+ /* This condition is actually a (temporary) hack for test suite
which
+* does not create capabilities cache */
+ if (driver-qemuCapsCache) {
+ virQEMUCapsPtr qemuCaps =
+ virQEMUCapsCacheLookup(driver-qemuCapsCache,
def-emulator);
+ addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX);
+ }
+ }
break;
case VIR_ARCH_PPC64:
--
1.9.5.msysgit.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] conf: Add helpers to insert/remove/find shmem devices in domain def
On 07/09/2015 02:09 PM, Martin Kletzander wrote:
On Thu, Jul 09, 2015 at 11:44:30AM +0800, lhuang wrote:
On 07/08/2015 08:14 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:
The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 61
src/conf/domain_conf.h | 7 ++
src/libvirt_private.syms | 3 +++
3 files changed, 71 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 306b718..8a8e4f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def,
}
+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+return VIR_APPEND_ELEMENT(def-shmems, def-nshmems, shmem);
+}
+
+
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+size_t i;
+
+for (i = 0; i def-nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def-shmems[i];
+
+ if (STRNEQ_NULLABLE(shmem-name, tmpshmem-name))
+ continue;
+
I think that you shouldn't be able to have two shmem/ elements in
the same domain, and since the name is mandatory, STREQ() should be
enough to check whether you need to return current 'i'.
Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one
guest could have more than one shmem/ element, maybe one is
non-server shmem and another is server shmem, it depends on how to use
it.
Maybe this function could have a bool parameter (something like a
'full_match') that would say whether everything must match or the name
is enough. And it the bool is false, streq is enough, but if it's
true, you could abstract the internals of this for body into
virDomainShmemEquals() or something and that would be called instead.
Good idea, this way is okay to me.
Thanks a lot for your opinion.
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cpu: Add support for MPX and AVX512 Intel features
Corresponding QEMU commits: MPX 79e9ebebbf2a00c46fcedb6dc7dd5e12bbd30216 AVX512 9aecd6f8aef653cea58932f06a2740299dbe5fd3 Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/cpu/cpu_map.xml | 15 +++ 1 file changed, 15 insertions(+) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index b9e95cf..b924bd3 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -317,6 +317,12 @@ feature name='rtm' cpuid function='0x0007' ebx='0x0800'/ /feature +feature name='mpx' + cpuid function='0x0007' ebx='0x4000'/ +/feature +feature name='avx512f' !-- AVX-512 Foundation -- + cpuid function='0x0007' ebx='0x0001'/ +/feature feature name='rdseed' cpuid function='0x0007' ebx='0x0004'/ /feature @@ -326,6 +332,15 @@ feature name='smap' cpuid function='0x0007' ebx='0x0010'/ /feature +feature name='avx512pf' !-- AVX-512 Prefetch -- + cpuid function='0x0007' ebx='0x0400'/ +/feature +feature name='avx512er' !-- AVX-512 Exponential and Reciprocal -- + cpuid function='0x0007' ebx='0x0800'/ +/feature +feature name='avx512cd' !-- AVX-512 Conflict Detection -- + cpuid function='0x0007' ebx='0x1000'/ +/feature !-- Advanced Power Management edx features -- feature name='invtsc' migratable='no' -- 2.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] qemuDomainEventQueue: Check if event is non-NULL
On Wed, Jul 08, 2015 at 19:36:05 +0200, Jiri Denemark wrote: Every single call to qemuDomainEventQueue() uses the following pattern: if (event) qemuDomainEventQueue(driver, event); Let's move the check for valid event to qemuDomainEventQueue and simplify all callers. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_blockjob.c | 6 ++-- src/qemu/qemu_cgroup.c| 3 +- src/qemu/qemu_domain.c| 6 ++-- src/qemu/qemu_driver.c| 87 --- src/qemu/qemu_hotplug.c | 26 ++ src/qemu/qemu_migration.c | 24 + src/qemu/qemu_process.c | 72 +-- 7 files changed, 78 insertions(+), 146 deletions(-) Yay \o/, finally this is taken care of. ACK Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] qemu: Queue events in migration Finish phase ASAP
On Wed, Jul 08, 2015 at 19:36:06 +0200, Jiri Denemark wrote:
For quite a long time we don't need to postpone queueing events until
the end of the function since we no longer have the big driver lock.
Let's make the code of qemuMigrationFinish simpler by queuing events at
the time we generate them.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae7433e..cc7754c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
...
@@ -5709,16 +5708,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
dom = virGetDomain(dconn, vm-def-name, vm-def-uuid);
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_RESUMED,
- VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
+qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+VIR_DOMAIN_EVENT_RESUMED,
+VIR_DOMAIN_EVENT_RESUMED_MIGRATED));
I dislike this formatting.
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
VIR_DOMAIN_PAUSED_USER);
-qemuDomainEventQueue(driver, event);
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
-
VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
+qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+VIR_DOMAIN_EVENT_SUSPENDED,
+VIR_DOMAIN_EVENT_SUSPENDED_PAUSED));
}
if (virDomainObjIsActive(vm)
ACK,
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 2/4] test-gconfig: Fix various leaks
Running test-gconfig under valgrind reports a few leaks that this commit fixes. --- tests/test-gconfig.c | 9 + 1 file changed, 9 insertions(+) diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c index 0eec53e..de09c24 100644 --- a/tests/test-gconfig.c +++ b/tests/test-gconfig.c @@ -48,6 +48,7 @@ static void check_xml(GVirConfigDomain *domain, const char *reference_file) xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(domain)); g_assert_cmpstr(xml, ==, reference_xml); g_free(xml); +g_free(reference_xml); } @@ -224,6 +225,8 @@ static void test_domain_os(void) gvir_config_domain_set_os(domain, NULL); os = gvir_config_domain_get_os(domain); g_assert(os == NULL); + +g_object_unref(G_OBJECT(domain)); } @@ -294,9 +297,13 @@ static void test_domain_cpu(void) NULL); topology = gvir_config_capabilities_cpu_get_topology(GVIR_CONFIG_CAPABILITIES_CPU(cpu)); g_assert(topology == NULL); +g_object_unref(G_OBJECT(cpu)); + gvir_config_domain_set_cpu(domain, NULL); cpu = gvir_config_domain_get_cpu(domain); g_assert(cpu == NULL); + +g_object_unref(G_OBJECT(domain)); } @@ -347,6 +354,7 @@ static void test_domain_device_disk(void) g_assert(gvir_config_domain_disk_driver_get_copy_on_read(driver)); g_assert_cmpint(gvir_config_domain_disk_get_target_bus(disk), ==, GVIR_CONFIG_DOMAIN_DISK_BUS_IDE); g_assert_cmpstr(gvir_config_domain_disk_get_target_dev(disk), ==, hda); +g_object_unref(G_OBJECT(driver)); gvir_config_domain_disk_set_driver(disk, NULL); driver = gvir_config_domain_disk_get_driver(disk); @@ -433,6 +441,7 @@ static void test_domain_device_input(void) GVIR_CONFIG_DOMAIN_INPUT_DEVICE_TABLET); gvir_config_domain_input_set_bus(input, GVIR_CONFIG_DOMAIN_INPUT_BUS_USB); gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(input)); +g_object_unref(G_OBJECT(input)); check_xml(domain, gconfig-domain-device-input.xml); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 4/4] gconfig: Use GVirConfigObject helpers for video XML
GVirConfigDomainVideo is using raw libxml calls to set the 'heads' and
'vram' XML attributes rather than the helpers provided by
GVirConfigObject. This commit changes that, making the code a bit
simpler.
---
libvirt-gconfig/libvirt-gconfig-domain-video.c | 38 +++---
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-video.c
b/libvirt-gconfig/libvirt-gconfig-domain-video.c
index 947d066..78ac54f 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-video.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-video.c
@@ -88,33 +88,27 @@ void
gvir_config_domain_video_set_model(GVirConfigDomainVideo *video,
void gvir_config_domain_video_set_vram(GVirConfigDomainVideo *video,
guint kbytes)
{
-xmlNodePtr node;
-char *vram_str;
+GVirConfigObject *node;
-node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(video));
-if (node == NULL)
-return;
-node = gvir_config_xml_get_element(node, model, NULL);
-if (node == NULL)
-return;
-vram_str = g_strdup_printf(%u, kbytes);
-xmlNewProp(node, (xmlChar*)vram, (xmlChar*)vram_str);
-g_free(vram_str);
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_VIDEO(video));
+node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(video), model);
+g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
+gvir_config_object_set_attribute_with_type(node, vram,
+ G_TYPE_UINT, kbytes,
+ NULL);
+g_object_unref(G_OBJECT(node));
}
void gvir_config_domain_video_set_heads(GVirConfigDomainVideo *video,
guint head_count)
{
-xmlNodePtr node;
-char *heads_str;
+GVirConfigObject *node;
-node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(video));
-if (node == NULL)
-return;
-node = gvir_config_xml_get_element(node, model, NULL);
-if (node == NULL)
-return;
-heads_str = g_strdup_printf(%u, head_count);
-xmlNewProp(node, (xmlChar*)heads, (xmlChar*)heads_str);
-g_free(heads_str);
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_VIDEO(video));
+node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(video), model);
+g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
+gvir_config_object_set_attribute_with_type(node, heads,
+ G_TYPE_UINT, head_count,
+ NULL);
+g_object_unref(G_OBJECT(node));
}
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/4] qemu: Refactor qemuCheckSharedDisk to create qemuCheckUnprivSGIO
On Wed, Jul 08, 2015 at 15:30:25 -0400, John Ferlan wrote: Split out the current function in order to share the code with hostdev in a future patch. Failure to match the expected sgio value against what is stored will cause an error which the caller would need to handle since only the caller has the disk (or eventually hostdev) specific data in order to uniquely identify the disk in an error message. Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_conf.c | 90 +++- 1 file changed, 61 insertions(+), 29 deletions(-) ACK, but this patch seems kind of useless without the followup patches that were dropped due to lack of upstream support. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/3] Introduce QEMU_CAPS_OBJECT_GPEX
This capability specifies that qemu can implement generic PCI host
controller. It is often used for virtual environments, including ARM.
Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 27686c3..0f37396 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
aarch64-off,
vhost-user-multiqueue, /* 190 */
+ gpex-pcihost,
);
@@ -1566,6 +1567,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ ivshmem, QEMU_CAPS_DEVICE_IVSHMEM },
{ pc-dimm, QEMU_CAPS_DEVICE_PC_DIMM },
{ pci-serial, QEMU_CAPS_DEVICE_PCI_SERIAL },
+{ gpex-pcihost, QEMU_CAPS_OBJECT_GPEX},
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 30aa504..711 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -230,6 +230,7 @@ typedef enum {
QEMU_CAPS_DEVICE_PCI_SERIAL = 188, /* -device pci-serial */
QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */
QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
+QEMU_CAPS_OBJECT_GPEX= 191, /* have generic PCI host controller */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
--
1.9.5.msysgit.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib PATCHv5 7/7] gobject: Add wrapper for virNetworkGetDHCPLeases
On Thu, Jul 9, 2015 at 8:05 AM, Christophe Fergeau cferg...@redhat.com wrote: On Wed, Jul 08, 2015 at 07:00:36PM +0100, Zeeshan Ali (Khattak) wrote: On Wed, Jul 8, 2015 at 3:42 PM, Christophe Fergeau cferg...@redhat.com wrote: but you haven't brought anything forward to support that ugly hack statement in this specific case, #ifdef based solution is going to be ugly, surely you know that. I never made any claims about the magnitude of ugliness, I always want to avoid ugly hacks whenever possible. If it just requires 2 or 3 #ifdef, adding them and forgetting they existed would have been faster than this thread ;) I agree but I want to clarify how we intend to proceed about this in future. I'm fine with 2 or 3 but with this kind of extremely strict definition of too new libvirt dep, we'd need to keep on doing this and before you know it, we'll have lots of these ugly hacks. nor any hard data regarding which distros could be impacted by a req bump. I'll stop this discussion until you bring some concrete datapoints to the table. Fair enough! The main point of this discussion was not to convince you but rather to get a third opinion. Honestly, this is a very weird attitude, rather than trying to come with hard facts, you prefer having some kind of poll and make an arbitary uninformed decision. I regret having to resort to some sort of poll as well but this is not a purely objective discussion. I did present arguments but you maintain that I did not. So let's leave at that, shall we? I'll just proceed as Dan tell me to. It's his project in the end. -- Regards, Zeeshan Ali (Khattak) Befriend GNOME: http://www.gnome.org/friends/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: Revert volume obj list updating after volume creation (4749d82a)
This patch reverts commit 4749d82a which tried to tweak the logic in
volume creation. We did realloc and update our object list before we executed
volume building within a specific storage backend. If that failed, we
had to update (again) our object list to the original state as it was before the
build and delete the volume from the pool (even though it didn't exist - this
truly depends on the backend).
I misunderstood the base idea to be able to poll the status of the volume
creation using vol-info. After commit 4749d82a this wasn't possible
anymore, although no BZ has been reported yet.
Commit 4749d82a also claimed to fix
https://bugzilla.redhat.com/show_bug.cgi?id=1223177, but commit c8be606b of the
same series as 4749d82ad (which was more of a refactor than a fix)
fixes the same issue so the revert should be pretty straightforward.
Further more, BZ https://bugzilla.redhat.com/show_bug.cgi?id=1241454 can be
fixed with this revert.
---
src/storage/storage_driver.c | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index d3cdbc5..b67a5d8 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1803,6 +1803,9 @@ storageVolCreateXML(virStoragePoolPtr obj,
goto cleanup;
}
+if (VIR_REALLOC_N(pool-volumes.objs,
+ pool-volumes.count+1) 0)
+goto cleanup;
if (!backend-createVol) {
virReportError(VIR_ERR_NO_SUPPORT,
@@ -1817,6 +1820,14 @@ storageVolCreateXML(virStoragePoolPtr obj,
if (backend-createVol(obj-conn, pool, voldef) 0)
goto cleanup;
+pool-volumes.objs[pool-volumes.count++] = voldef;
+volobj = virGetStorageVol(obj-conn, pool-def-name, voldef-name,
+ voldef-key, NULL, NULL);
+if (!volobj) {
+pool-volumes.count--;
+goto cleanup;
+}
+
if (VIR_ALLOC(buildvoldef) 0) {
voldef = NULL;
goto cleanup;
@@ -1845,18 +1856,15 @@ storageVolCreateXML(virStoragePoolPtr obj,
voldef-building = false;
pool-asyncjobs--;
-if (buildret 0)
+if (buildret 0) {
+VIR_FREE(buildvoldef);
+storageVolDeleteInternal(volobj, backend, pool, voldef,
+ 0, false);
+voldef = NULL;
goto cleanup;
-}
-
-if (VIR_REALLOC_N(pool-volumes.objs,
- pool-volumes.count+1) 0)
-goto cleanup;
+}
-pool-volumes.objs[pool-volumes.count++] = voldef;
-if (!(volobj = virGetStorageVol(obj-conn, pool-def-name, voldef-name,
-voldef-key, NULL, NULL)))
-goto cleanup;
+}
if (backend-refreshVol
backend-refreshVol(obj-conn, pool, voldef) 0)
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Revert volume obj list updating after volume creation (4749d82a)
On Thu, Jul 09, 2015 at 01:48:16PM +0200, Erik Skultety wrote: This patch reverts commit 4749d82a which tried to tweak the logic in volume creation. We did realloc and update our object list before we executed volume building within a specific storage backend. If that failed, we had to update (again) our object list to the original state as it was before the build and delete the volume from the pool (even though it didn't exist - this truly depends on the backend). I misunderstood the base idea to be able to poll the status of the volume creation using vol-info. After commit 4749d82a this wasn't possible anymore, although no BZ has been reported yet. Commit 4749d82a also claimed to fix https://bugzilla.redhat.com/show_bug.cgi?id=1223177, but commit c8be606b of the same series as 4749d82ad (which was more of a refactor than a fix) fixes the same issue so the revert should be pretty straightforward. Further more, BZ https://bugzilla.redhat.com/show_bug.cgi?id=1241454 can be fixed with this revert. --- src/storage/storage_driver.c | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Revert volume obj list updating after volume creation (4749d82a)
On 09/07/15 14:13, Ján Tomko wrote: On Thu, Jul 09, 2015 at 01:48:16PM +0200, Erik Skultety wrote: This patch reverts commit 4749d82a which tried to tweak the logic in volume creation. We did realloc and update our object list before we executed volume building within a specific storage backend. If that failed, we had to update (again) our object list to the original state as it was before the build and delete the volume from the pool (even though it didn't exist - this truly depends on the backend). I misunderstood the base idea to be able to poll the status of the volume creation using vol-info. After commit 4749d82a this wasn't possible anymore, although no BZ has been reported yet. Commit 4749d82a also claimed to fix https://bugzilla.redhat.com/show_bug.cgi?id=1223177, but commit c8be606b of the same series as 4749d82ad (which was more of a refactor than a fix) fixes the same issue so the revert should be pretty straightforward. Further more, BZ https://bugzilla.redhat.com/show_bug.cgi?id=1241454 can be fixed with this revert. --- src/storage/storage_driver.c | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) ACK Jan Thanks, now pushed. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] qemu: Don't fail migration on save status failure
On Wed, Jul 08, 2015 at 19:36:02 +0200, Jiri Denemark wrote: When we save status XML at the point during migration where we have already started the domain on destination, we can't really go back and abort migration. Thus the only thing we can do is to log a warning and report success. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_migration.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib PATCHv5 7/7] gobject: Add wrapper for virNetworkGetDHCPLeases
On Thu, Jul 09, 2015 at 12:42:23PM +0100, Zeeshan Ali (Khattak) wrote: Honestly, this is a very weird attitude, rather than trying to come with hard facts, you prefer having some kind of poll and make an arbitary uninformed decision. I regret having to resort to some sort of poll as well but this is not a purely objective discussion. I did present arguments but you maintain that I did not. So let's leave at that, shall we? Unless I missed important things, your arguments boiled down to one year is old enough, we should not care about distros. I've asked what the impact would be on distros, no answer so far. I've asked what the impact would be in terms of #ifdef ugliness, no answer so far. Quite hard to make a decision with so few details ;) I'll just proceed as Dan tell me to. It's his project in the end. danpb is away for a while as I understand it, and my feeling is that we could resolve this on our own if you were at least trying to give more details about the various alternatives. Christophe pgpRmCiOOM6ng.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] shall libvirtd validate guest's name ?
linux-ZyvZnF:~ # virsh list --all IdName State - redhat7;reboot shut off - oscar-vm-5 shut off As shown above, 1 we use command virsh define a.xml to define a guest with a name containing ';', that's 'redhat7;reboot' 2 then we start the guest: virsh start redhat7;reboot 3 shell consider the command as a) run virsh start redhat7, failed b) run reboot, to reboot the host And *the host get rebooted*. shall libvirtd do the guest-name-validation work? Or other suggustions? -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 2/3] buildsys: Add missing libraries to LDFLAGS
Without these changes 'make check' would not run on Ubuntu because of a link failure as the tests are directly using glib/libvirt symbols. --- tests/Makefile.am | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 63865e8..c146817 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -12,7 +12,10 @@ AM_CFLAGS = \ LDADD = \ $(top_builddir)/libvirt-gconfig/libvirt-gconfig-1.0.la \ - $(top_builddir)/libvirt-glib/libvirt-glib-1.0.la + $(top_builddir)/libvirt-glib/libvirt-glib-1.0.la \ + $(LIBVIRT_LIBS) \ + $(GLIB2_LIBS) \ + $(GOBJECT2_LIBS) test_programs = test-gconfig test-events -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 1/3] Add LibvirtGConfigDomainChardevSourceUnix
This is needed to be able to add UNIX channels
---
libvirt-gconfig/Makefile.am| 2 +
.../libvirt-gconfig-domain-chardev-source-unix.c | 84 ++
.../libvirt-gconfig-domain-chardev-source-unix.h | 68 ++
libvirt-gconfig/libvirt-gconfig.h | 1 +
libvirt-gconfig/libvirt-gconfig.sym| 7 ++
libvirt-gconfig/tests/test-domain-create.c | 14
tests/test-gconfig.c | 11 +++
tests/xml/gconfig-domain-device-channel.xml| 3 +
8 files changed, 190 insertions(+)
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-chardev-source-unix.c
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-chardev-source-unix.h
diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
index a9a6591..77b2032 100644
--- a/libvirt-gconfig/Makefile.am
+++ b/libvirt-gconfig/Makefile.am
@@ -32,6 +32,7 @@ GCONFIG_HEADER_FILES = \
libvirt-gconfig-domain-chardev-source-pty.h \
libvirt-gconfig-domain-chardev-source-spiceport.h \
libvirt-gconfig-domain-chardev-source-spicevmc.h \
+ libvirt-gconfig-domain-chardev-source-unix.h \
libvirt-gconfig-domain-clock.h \
libvirt-gconfig-domain-console.h \
libvirt-gconfig-domain-controller.h \
@@ -122,6 +123,7 @@ GCONFIG_SOURCE_FILES = \
libvirt-gconfig-domain-chardev-source-pty.c \
libvirt-gconfig-domain-chardev-source-spiceport.c \
libvirt-gconfig-domain-chardev-source-spicevmc.c \
+ libvirt-gconfig-domain-chardev-source-unix.c \
libvirt-gconfig-domain-clock.c \
libvirt-gconfig-domain-console.c \
libvirt-gconfig-domain-controller.c \
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-unix.c
b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-unix.c
new file mode 100644
index 000..162b788
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-unix.c
@@ -0,0 +1,84 @@
+/*
+ * libvirt-gconfig-domain-chardev-source-unix.c: libvirt domain chardev unix
configuration
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2015 T A Mahadevan
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: T A Mahadevan ta.mahade...@gmail.com
+ */
+
+#include config.h
+
+#include libvirt-gconfig/libvirt-gconfig.h
+#include libvirt-gconfig/libvirt-gconfig-private.h
+
+#define GVIR_CONFIG_DOMAIN_CHARDEV_SOURCE_UNIX_GET_PRIVATE(obj)
\
+(G_TYPE_INSTANCE_GET_PRIVATE((obj),
GVIR_CONFIG_TYPE_DOMAIN_CHARDEV_SOURCE_UNIX,
GVirConfigDomainChardevSourceUnixPrivate))
+
+struct _GVirConfigDomainChardevSourceUnixPrivate
+{
+gboolean unused;
+};
+
+G_DEFINE_TYPE(GVirConfigDomainChardevSourceUnix,
gvir_config_domain_chardev_source_unix, GVIR_CONFIG_TYPE_DOMAIN_CHARDEV_SOURCE);
+
+
+static void
gvir_config_domain_chardev_source_unix_class_init(GVirConfigDomainChardevSourceUnixClass
*klass)
+{
+g_type_class_add_private(klass,
sizeof(GVirConfigDomainChardevSourceUnixPrivate));
+}
+
+
+static void
gvir_config_domain_chardev_source_unix_init(GVirConfigDomainChardevSourceUnix
*source)
+{
+g_debug(Init GVirConfigDomainChardevSourceUnix=%p, source);
+
+source-priv = GVIR_CONFIG_DOMAIN_CHARDEV_SOURCE_UNIX_GET_PRIVATE(source);
+}
+
+
+GVirConfigDomainChardevSourceUnix
*gvir_config_domain_chardev_source_unix_new(void)
+{
+GVirConfigObject *object;
+
+/* the name of the root node is just a placeholder, it will be
+ * overwritten when the GVirConfigDomainChardevSourceUnix is attached to a
+ * GVirConfigDomainChardev
+ */
+object =
gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_CHARDEV_SOURCE_UNIX, dummy,
NULL);
+gvir_config_object_set_attribute(object, type, unix, NULL);
+return GVIR_CONFIG_DOMAIN_CHARDEV_SOURCE_UNIX(object);
+}
+
+
+GVirConfigDomainChardevSourceUnix
*gvir_config_domain_chardev_source_unix_new_from_xml(const gchar *xml,
+
GError **error)
+{
+GVirConfigObject *object;
+
+/* the
[libvirt] [libvirt-glib 3/3] Add ram and vgamem attributes for graphics model.
---
libvirt-gconfig/libvirt-gconfig-domain-video.c | 25 +
libvirt-gconfig/libvirt-gconfig-domain-video.h | 5 +
libvirt-gconfig/libvirt-gconfig.sym| 2 ++
3 files changed, 32 insertions(+)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-video.c
b/libvirt-gconfig/libvirt-gconfig-domain-video.c
index 947d066..cc2034d 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-video.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-video.c
@@ -102,6 +102,31 @@ void
gvir_config_domain_video_set_vram(GVirConfigDomainVideo *video,
g_free(vram_str);
}
+void gvir_config_domain_video_set_ram(GVirConfigDomainVideo *video,
+ guint kbytes)
+{
+GVirConfigObject *node;
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_VIDEO(video));
+node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(video), model);
+g_return_if_fail(GVIR_CONFIG_OBJECT(node));
+gvir_config_object_set_attribute_with_type(node, ram, G_TYPE_UINT,
+ kbytes, NULL);
+g_object_unref(G_OBJECT(node));
+}
+
+
+void gvir_config_domain_video_set_vgamem(GVirConfigDomainVideo *video,
+ guint kbytes)
+{
+GVirConfigObject *node;
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_VIDEO(video));
+node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(video), model);
+g_return_if_fail(GVIR_CONFIG_OBJECT(node));
+gvir_config_object_set_attribute_with_type(node, vgamem, G_TYPE_UINT,
+ kbytes, NULL);
+g_object_unref(G_OBJECT(node));
+}
+
void gvir_config_domain_video_set_heads(GVirConfigDomainVideo *video,
guint head_count)
{
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-video.h
b/libvirt-gconfig/libvirt-gconfig-domain-video.h
index f83d5aa..a87ec4f 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-video.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-video.h
@@ -74,6 +74,11 @@ void
gvir_config_domain_video_set_model(GVirConfigDomainVideo *video,
GVirConfigDomainVideoModel model);
void gvir_config_domain_video_set_vram(GVirConfigDomainVideo *video,
guint kbytes);
+
+void gvir_config_domain_video_set_ram(GVirConfigDomainVideo *video,
+ guint kbytes);
+void gvir_config_domain_video_set_vgamem(GVirConfigDomainVideo *video,
+ guint kbytes);
void gvir_config_domain_video_set_heads(GVirConfigDomainVideo *video,
guint head_count);
diff --git a/libvirt-gconfig/libvirt-gconfig.sym
b/libvirt-gconfig/libvirt-gconfig.sym
index 6267197..89dd589 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -729,6 +729,8 @@ global:
gvir_config_domain_chardev_source_unix_get_type;
gvir_config_domain_chardev_source_unix_new;
gvir_config_domain_chardev_source_unix_new_from_xml;
+ gvir_config_domain_video_set_ram;
+ gvir_config_domain_video_set_vgamem;
} LIBVIRT_GCONFIG_0.2.1;
# define new API here using predicted next version number
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] qemu: Split qemuMigrationFinish
On Wed, Jul 08, 2015 at 19:36:00 +0200, Jiri Denemark wrote:
Separate code which makes incoming domain persistent into
qemuMigrationPersist.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 75
++-
1 file changed, 48 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 97901c6..6b06e79 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5526,6 +5526,51 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr
def)
}
+static int
+qemuMigrationPersist(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuMigrationCookiePtr mig)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virCapsPtr caps = NULL;
+virDomainDefPtr vmdef;
+virObjectEventPtr event;
+bool newVM;
+int ret = -1;
+
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+newVM = !vm-persistent;
+vm-persistent = 1;
+
+if (mig-persistent)
+vm-newDef = mig-persistent;
This could be done unconditionally since vm-newDef will apperently be
unset.
+
+vmdef = virDomainObjGetPersistentDef(caps, driver-xmlopt, vm);
+if (!vmdef)
+goto cleanup;
+
+if (virDomainSaveConfig(cfg-configDir, vmdef) 0)
+goto cleanup;
+
+event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_DEFINED,
+ newVM ?
+ VIR_DOMAIN_EVENT_DEFINED_ADDED :
+ VIR_DOMAIN_EVENT_DEFINED_UPDATED);
+if (event)
+qemuDomainEventQueue(driver, event);
+
+ret = 0;
+
+ cleanup:
+virObjectUnref(caps);
+virObjectUnref(cfg);
+return ret;
+}
+
+
virDomainPtr
qemuMigrationFinish(virQEMUDriverPtr driver,
virConnectPtr dconn,
ACK as-is.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/7] qemu: Don't report false errors in migration protocol v2
On Wed, Jul 08, 2015 at 19:36:04 +0200, Jiri Denemark wrote: Finish is the final state in v2 of our migration protocol. If something fails, we have no option to abort the migration and resume the original domain. Non fatal errors (such as failure to start guest CPUs or make the domain persistent) has to be treated as success. Keeping the domain running while reporting the failure was just asking for troubles. s/es/e/ Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_migration.c | 20 +++- 1 file changed, 7 insertions(+), 13 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] qemu: Simplify qemuMigrationFinish
On Wed, Jul 08, 2015 at 19:36:01 +0200, Jiri Denemark wrote:
Offline migration migration is quite special because we don't really
need to do anything but make the domain persistent. Let's do it
separately from normal migration to avoid cluttering the code with
!(flags VIR_MIGRATE_OFFLINE).
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 72
+++
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6b06e79..6a3e2e6 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5625,8 +5625,14 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
/* Did the migration go as planned? If yes, return the domain
* object, but if no, clean up the empty qemu process.
*/
-if (retcode == 0) {
-if (!virDomainObjIsActive(vm) !(flags VIR_MIGRATE_OFFLINE)) {
+if (flags VIR_MIGRATE_OFFLINE) {
+if (retcode != 0 ||
+qemuMigrationPersist(driver, vm, mig) 0)
+goto endjob;
This isn't entirely equivalend to the previous impl, since if
qemuMigrationPersist fails at a later stage the vm-persistent flag does
get set. The code below cleared it in some cases, but in this case it
would not get cleared.
Either qemuMigrationPersist should get updated so that the flag is set
only when everything went well, or you should clear if afterwards.
Later on in the endjob section there is a check if the object is active
so you'd get a persistent VM object with no definition in some cases.
+
+dom = virGetDomain(dconn, vm-def-name, vm-def-uuid);
+} else if (retcode == 0) {
+if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(guest unexpectedly quit));
qemuMigrationErrorReport(driver, vm-def-name);
ACK if you fix the offline migration error handling.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/7] qemu: Kill domain when migration finish fails
On Wed, Jul 08, 2015 at 19:36:03 +0200, Jiri Denemark wrote:
Whenever something fails during incoming migration in Finish phase
before we started guest CPUs, we need to kill the domain in addition to
reporting the failure.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 32
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9439954..576b32d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5589,6 +5589,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm-privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
unsigned short port;
+bool keep = false;
VIR_DEBUG(driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d,
cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d,
@@ -5647,15 +5648,9 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
}
}
-if (qemuMigrationVPAssociatePortProfiles(vm-def) 0) {
-qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
-VIR_QEMU_PROCESS_STOP_MIGRATED);
-virDomainAuditStop(vm, failed);
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
-
VIR_DOMAIN_EVENT_STOPPED_FAILED);
+if (qemuMigrationVPAssociatePortProfiles(vm-def) 0)
goto endjob;
-}
+
if (mig-network qemuDomainMigrateOPDRelocate(driver, vm, mig)
0)
VIR_WARN(unable to provide network data for relocation);
@@ -5681,11 +5676,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* to restart during confirm() step, so we kill it off now.
*/
if (v3proto) {
-qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
-VIR_QEMU_PROCESS_STOP_MIGRATED);
-virDomainAuditStop(vm, failed);
if (newVM)
vm-persistent = 0;
If you choose to fix qemuMigrationPersist so that it sets the persistent
flag only on succes to fix the previous patch then this will create a
conflict since the above statement shouldn't be necessary.
+} else {
+keep = true;
}
goto endjob;
}
ACK,
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib PATCHv5 7/7] gobject: Add wrapper for virNetworkGetDHCPLeases
On Thu, Jul 9, 2015 at 12:56 PM, Christophe Fergeau cferg...@redhat.com wrote: On Thu, Jul 09, 2015 at 12:42:23PM +0100, Zeeshan Ali (Khattak) wrote: Honestly, this is a very weird attitude, rather than trying to come with hard facts, you prefer having some kind of poll and make an arbitary uninformed decision. I regret having to resort to some sort of poll as well but this is not a purely objective discussion. I did present arguments but you maintain that I did not. So let's leave at that, shall we? Unless I missed important things, your arguments boiled down to one year is old enough, we should not care about distros. It's very hard to discuss if you over-exagerate my opinions to make them sound bad. I'm did not say we should not care about distros but rather that 1 year is plenty of care. I've asked what the impact would be on distros, no answer so far. The answer was that 1 year of grace time is enough and we shouldn't need to be bound by release cycles and packaging of individual distros. Also I pointed out the fact that it does not (in practical terms) make any sense to only upgrade to latest libvirt-glib but wanting to stay with an year old libvirt. I've asked what the impact would be in terms of #ifdef ugliness, no answer so far. The answer was that magnitude of ugliness is irrelevant. Surely you know that this is not anything objective. To me a single #ifdef is ugly enough but seems it's not for you so how do you expect either of us to convince each other with arguments? If you really need something more concrete, I see at least 12 #ifdefs needed. Pretty ugly for my taste at least. Quite hard to make a decision with so few details ;) All needed details are avaiable and it's only a matter of taste and amount of care we want to give to distros. Both are subjective matters. I'll just proceed as Dan tell me to. It's his project in the end. danpb is away for a while as I understand it, and my feeling is that we could resolve this on our own if you were at least trying to give more details about the various alternatives. I don't know many alternatives. It's either bumping the dep or adding ugly #ifdef based solution. -- Regards, Zeeshan Ali (Khattak) Befriend GNOME: http://www.gnome.org/friends/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] Introduce virHashLockable
On Wed, Jul 08, 2015 at 15:22:49 +0200, Jiri Denemark wrote: This is a self-locking wrapper around virHashTable. Only a limited set of APIs are implemented now (the ones which are used in the following patch) as more can be added on demand. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/libvirt_private.syms | 3 ++ src/util/virhash.c | 81 src/util/virhash.h | 10 ++ 3 files changed, 94 insertions(+) ACK although I dislike the Lockable part since you are not able to lock that hash, but that hash is self-locking. But I don't have a better name suggestion. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] qemu_hotplug: try harder to eject media
On 07/03/2015 09:51 AM, Pavel Hrdina wrote:
On Wed, Jul 01, 2015 at 05:39:49PM -0400, John Ferlan wrote:
On 06/29/2015 11:17 AM, Pavel Hrdina wrote:
Some guests lock the tray and QEMU eject command will simply fail to
eject the media. But the guest OS can handle this attempt to eject the
media and can unlock the tray and open it. In this case, we should try
again to actually eject the media.
If the first attempt fails to detect a tray_open we will fail with
error, from monitor. If we receive that event, we know, that the guest
properly reacted to the eject request, unlocked the tray and opened it.
In this case, we need to run the command again to actually eject the
media from the device. The reason to call it again is, that QEMU don't
s/don't/doesn't
wait for the guest to react and report an error, that the tray is
locked.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1147471
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_hotplug.c | 73
+++--
src/qemu/qemu_process.c | 2 ++
2 files changed, 36 insertions(+), 39 deletions(-)
I suppose if I had gone back to the commit message and reread it in
light of what I was reading in the qemu-devel message linked in the bz
and what I saw in the code, then perhaps it would all make sense. Of
course the text you add below now makes it clear what the order of
processing is and why the while loop is used.
Personally I think this is a case where that commit message could be
placed into the code to make it clearer what is being done and why. I
know there are those against that because it's in the commit message or
perhaps some linked bug, but if I'm reading code and wondering what it
does, I'm not necessarily going back to the commit/bug that added the
code (OK, at least not at first).
Whether you add a comment into the code to indicate why this while loop
is important and how the processing works due to the event and locked
media...
ACK series,
John
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0628964..17595b7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -59,7 +59,7 @@
VIR_LOG_INIT(qemu.qemu_hotplug);
-#define CHANGE_MEDIA_RETRIES 10
+#define CHANGE_MEDIA_TIMEOUT 5000
/* Wait up to 5 seconds for device removal to finish. */
unsigned long long qemuDomainRemoveDeviceWaitTime = 1000ull * 5;
@@ -166,12 +166,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr
driver,
virStorageSourcePtr newsrc,
bool force)
{
-int ret = -1;
+int ret = -1, rc;
char *driveAlias = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
-int retries = CHANGE_MEDIA_RETRIES;
const char *format = NULL;
char *sourcestr = NULL;
+bool ejectRetry = false;
+unsigned long long now;
if (!disk-info.alias) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -193,36 +194,31 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr
driver,
if (!(driveAlias = qemuDeviceDriveHostAlias(disk, priv-qemuCaps)))
goto error;
-qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorEjectMedia(priv-mon, driveAlias, force);
-if (qemuDomainObjExitMonitor(driver, vm) 0) {
-ret = -1;
-goto cleanup;
-}
-
-if (ret 0)
-goto error;
+do {
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorEjectMedia(priv-mon, driveAlias, force);
+if (qemuDomainObjExitMonitor(driver, vm) 0)
+goto cleanup;
-virObjectRef(vm);
-/* we don't want to report errors from media tray_open polling */
-while (retries) {
-if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
-break;
+if (rc == -2) {
+/* we've already tried, error out */
+if (ejectRetry)
+goto error;
+VIR_DEBUG(tray is locked, wait for the guest to unlock
+ the tray and try to eject it again);
+ejectRetry = true;
+} else if (rc 0) {
+goto error;
+}
-retries--;
-virObjectUnlock(vm);
-VIR_DEBUG(Waiting 500ms for tray to open. Retries left %d,
retries);
-usleep(500 * 1000); /* sleep 500ms */
-virObjectLock(vm);
-}
-virObjectUnref(vm);
+if (virTimeMillisNow(now) 0)
+goto error;
-if (retries = 0) {
-virReportError(VIR_ERR_OPERATION_FAILED, %s,
- _(Unable to eject media));
-ret = -1;
-goto error;
-}
+while (disk-tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) {
+if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0)
+goto error;
+}
Seems this should be
if (rc == -2) wait for TRAY_MOVED (need a new event)
Re: [libvirt] Entering freeze for libvirt-1.2.17
On 2015-06-30 2:49 AM, Daniel Veillard wrote: On Tue, Jun 30, 2015 at 11:00:24AM +0200, Guido Günther wrote: On Tue, Jun 30, 2015 at 03:00:09PM +0800, Daniel Veillard wrote: On Mon, Jun 29, 2015 at 10:34:55PM +0200, Guido Günther wrote: On Sun, Jun 28, 2015 at 01:00:01PM +0800, Daniel Veillard wrote: Following discussions on Friday, I applied the patches to deactivate the subset of Admin APIs and revert from 1.3.0 to 1.2.17. I then tagged in git and pushed signed tarballs and rpms to the usual place: ftp://libvirt.org/pub/libvirt/ I didn't run my usual tests on that one, my infra is in flux, so even more reasons for people to give it a try :-) I'm likely to make a candidate release 2 on Tuesday and if all goes well we can push 1.2.17 on Thursday, Building the tarball fails for me with: make[4]: Entering directory '/tmp/buildd/libvirt-1.2.17~rc1/debian/build/docs' missing XHTML1 DTD cat: internals/locking.html.tmp: No such file or directory Makefile:2385: recipe for target 'internals/locking.html' failed make[4]: *** [internals/locking.html] Error 1 The missing XHTML1 DTD just means you can't validate the locking.html.tmp against a local copy of the DTD for XHTML1, but then it seems that the locking.html.tmp wasn't generated. It should be generated via xsltproc, it seems it's missing in your build environment, make sure you have it. Make sure you have xmllint, xsltproc and xhtml1-dtds in your build system, I should have added that I tried this with and without xsltproc + xmllint. I now also added the DTDs but no change (the build env didn't change since the last release). humpf ... locking.html.in wasn't touched since Feb, that need more attention and building from the tarballs worked here, strange Daniel http://libvirt.org/git/?p=libvirt.git;a=commit;h=1310b1358cdf9c8acba6e0e85feb869241e59faa I had to revert this commit to get 1.2.17 to build under debian packaging chroot. -Peter smime.p7s Description: S/MIME Cryptographic Signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.2.17
On 2015-07-09 2:00 PM, Peter Kieser wrote: On 2015-06-30 2:49 AM, Daniel Veillard wrote: On Tue, Jun 30, 2015 at 11:00:24AM +0200, Guido Günther wrote: On Tue, Jun 30, 2015 at 03:00:09PM +0800, Daniel Veillard wrote: On Mon, Jun 29, 2015 at 10:34:55PM +0200, Guido Günther wrote: On Sun, Jun 28, 2015 at 01:00:01PM +0800, Daniel Veillard wrote: Following discussions on Friday, I applied the patches to deactivate the subset of Admin APIs and revert from 1.3.0 to 1.2.17. I then tagged in git and pushed signed tarballs and rpms to the usual place: ftp://libvirt.org/pub/libvirt/ I didn't run my usual tests on that one, my infra is in flux, so even more reasons for people to give it a try :-) I'm likely to make a candidate release 2 on Tuesday and if all goes well we can push 1.2.17 on Thursday, Building the tarball fails for me with: make[4]: Entering directory '/tmp/buildd/libvirt-1.2.17~rc1/debian/build/docs' missing XHTML1 DTD cat: internals/locking.html.tmp: No such file or directory Makefile:2385: recipe for target 'internals/locking.html' failed make[4]: *** [internals/locking.html] Error 1 The missing XHTML1 DTD just means you can't validate the locking.html.tmp against a local copy of the DTD for XHTML1, but then it seems that the locking.html.tmp wasn't generated. It should be generated via xsltproc, it seems it's missing in your build environment, make sure you have it. Make sure you have xmllint, xsltproc and xhtml1-dtds in your build system, I should have added that I tried this with and without xsltproc + xmllint. I now also added the DTDs but no change (the build env didn't change since the last release). humpf ... locking.html.in wasn't touched since Feb, that need more attention and building from the tarballs worked here, strange Daniel http://libvirt.org/git/?p=libvirt.git;a=commit;h=1310b1358cdf9c8acba6e0e85feb869241e59faa I had to revert this commit to get 1.2.17 to build under debian packaging chroot. -Peter As well as: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=c0b7d3126be18bea0ce5dcead7bab925bc17cfc5 -Peter smime.p7s Description: S/MIME Cryptographic Signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/5] Add support for migration event
On Wed, Jul 08, 2015 at 18:39:36 +0200, Jiri Denemark wrote: The final version of migration event patches which got accepted upstream had to be slightly modified -- the event is only emitted when events migration capability is turned on (default is off). Thus a new patch had to be added to libvirt. I'm resending all migration event patches for context even though they haven't changed at all since they were ACKed. Jiri Denemark (5): qemu_monitor: Wire up MIGRATION event qemu: Enable migration events on QMP monitor qemuDomainGetJobStatsInternal: Support migration events qemu: Update migration state according to MIGRATION event qemu: Wait for migration events on domain condition I did s/virDomainObjSignal/virDomainObjBroadcast/ required after Pavel's series and pushed these patches. Thanks for the reviews. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Log all arguments of qemuProcessStart
On Wed, Jul 08, 2015 at 09:44:09 +0200, Peter Krempa wrote: On Wed, Jul 08, 2015 at 09:31:03 +0200, Jiri Denemark wrote: Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_process.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) ACK, Pushed, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't use initialized ret in qemuRemoveSharedDevice
On 07/09/2015 01:28 PM, Peter Krempa wrote: On Thu, Jul 09, 2015 at 19:22:30 +0200, Guido Günther wrote: This fixes CC qemu/libvirt_driver_qemu_impl_la-qemu_conf.lo qemu/qemu_conf.c: In function 'qemuRemoveSharedDevice': qemu/qemu_conf.c:1384:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] --- This is only catched by Debian Wheezy's gcc 4.7.2, Jessies gcc 4.9.2 doesn't trap on it. src/qemu/qemu_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Oops. I've missed that in my review. ACK if you didn't push this already. Well it was there in the patch that got removed sigh http://www.redhat.com/archives/libvir-list/2015-July/msg00209.html Sorry about that John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Drop LFs at the end of error from QEMU log
On Wed, Jul 08, 2015 at 10:22:06 +0200, Peter Krempa wrote: On Wed, Jul 08, 2015 at 10:14:16 +0200, Jiri Denemark wrote: Libvirt's error messages do not end with a LF. However, when reading the error from QEMU log, we would read the LF from the log and keep it in the message. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_monitor.c | 3 +++ 1 file changed, 3 insertions(+) ACK, Pushed, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: Don't report false error from MigrateFinish
On Wed, Jul 08, 2015 at 15:22:51 +0200, Jiri Denemark wrote: virDomainMigrateFinish* APIs were unfortunately designed to return the pointer to the domain on destination and NULL on error. This looks OK in normal cases but the same API is also called when we know migration failed and thus we expect Finish to return NULL even if it actually did all it was supposed to do without any error. The call is defined to return nonnull domain pointer over RPC, which means returning NULL will always result in an error being send. If this was not in fact an error, the API itself wouldn't set anything to the thread local virError, which makes the RPC layer come up with it's own Library function returned error but did not set virError error. This is quite confusing and also hard to detect by the caller. This patch adds a special error code which can be used to check that Finish successfully aborted migration. Signed-off-by: Jiri Denemark jdene...@redhat.com --- include/libvirt/virterror.h | 1 + src/qemu/qemu_migration.c | 6 ++ src/util/virerror.c | 3 +++ 3 files changed, 10 insertions(+) I'm kind of not sure whether I like this solution or not. I understand the reasons for having it but I'm not liking it. ACK but I'd prefer another opinion on this if possible. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: Use error from Finish instead of unexpectedly failed
On Wed, Jul 08, 2015 at 15:22:52 +0200, Jiri Denemark wrote:
When QEMU exits on destination during migration, the source reports
either success (if the failure happened at the very end) or unhelpful
unexpectedly failed error message. However, the Finish API called on
the destination may report a real error so let's use it instead of the
generic one.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/libvirt-domain.c | 21 +++--
src/qemu/qemu_migration.c | 30 --
2 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 909c264..f18fee2 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3175,8 +3175,25 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
(dconn, dname, cookiein, cookieinlen, cookieout, cookieoutlen,
NULL, uri, destflags, cancelled);
}
-if (cancelled ddomain)
-VIR_ERROR(_(finish step ignored that migration was cancelled));
+
See below for comments:
+if (cancelled) {
+if (ddomain)
+VIR_ERROR(_(finish step ignored that migration was cancelled));
+
+/* If Finish reported a useful error, use it instead of the original
+ * migration unexpectedly failed error.
+ */
+if (orig_err
+orig_err-domain == VIR_FROM_QEMU
+orig_err-code == VIR_ERR_OPERATION_FAILED) {
+virErrorPtr err = virGetLastError();
+if (err-domain == VIR_FROM_QEMU
+err-code != VIR_ERR_MIGRATE_FINISH_OK) {
+virFreeError(orig_err);
+orig_err = NULL;
+}
+}
+}
/* If ddomain is NULL, then we were unable to start
* the guest on the target, and must restart on the
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3548d73..d02a0c6 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4957,8 +4957,25 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver,
dconnuri, uri, destflags, cancelled);
qemuDomainObjExitRemote(vm);
}
-if (cancelled ddomain)
-VIR_ERROR(_(finish step ignored that migration was cancelled));
+
The control flow below is weird. Here are a few facts from the code
above:
- ddomain is set via the domainMigrateFinish3(Params) and not anywhere
else
- domainMigrateFinish3 either reports an error or returns ddomain
thus:
+if (cancelled) {
+if (ddomain)
+VIR_ERROR(_(finish step ignored that migration was cancelled));
+
Basically all the code below is an else section to the above if
statement due to the previous fact, so ... the below code makes it kind
of opaque.
+/* If Finish reported a useful error, use it instead of the original
+ * migration unexpectedly failed error.
+ */
+if (orig_err
+orig_err-domain == VIR_FROM_QEMU
+orig_err-code == VIR_ERR_OPERATION_FAILED) {
The code check isn't robust enough IMO. If you want to keep it, the fact
that this happens in a way like this should be noted in a comment for
doNativeMigrate/doTunnelMigrate that set the errors.
+virErrorPtr err = virGetLastError();
And additionally there is no error reported that this could possibly
overwrite in case where ddomain is not NULL except for the one reported
in virTypedParamsReplaceString but I doubt that will be a better one.
(If that is the intention a comment would really be helpful.
+if (err-domain == VIR_FROM_QEMU
+err-code != VIR_ERR_MIGRATE_FINISH_OK) {
+virFreeError(orig_err);
+orig_err = NULL;
+}
+}
+}
/* If ddomain is NULL, then we were unable to start
* the guest on the target, and must restart on the
Otherwise makes sense. I'd like to either have an explanation of the
above control flow or a fixed version.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: set dom0 state to running
Commit 45697fe5 added dom0 to driver-domains, but missed setting its state to 'running' virsh list IdName State 0 Domain-0 shut off Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/libxl/libxl_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index e72b12d..5f69b49 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -549,6 +549,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver) def = NULL; +virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); vm-def-vcpus = d_info.vcpu_online; vm-def-maxvcpus = d_info.vcpu_max_id + 1; vm-def-mem.cur_balloon = d_info.current_memkb; -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
