[libvirt] [PATCH V3 0/3] libxl: domain destroy fixes
V3 of a small series to fix issues wrt domain destroy V1: https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html V2: https://www.redhat.com/archives/libvir-list/2015-April/msg00072.html In this version, patch 3 is changed a bit to provide a wrapper for unlocking, destroying, and locking the domain. Existing libxl_domain_destroy callers are changed to use the wrapper. Jim Fehlig (3): libxl: Move job acquisition in libxlDomainStart to callers libxl: acquire a job when destroying a domain libxl: drop virDomainObj lock when destroying a domain src/libxl/libxl_domain.c| 106 ++-- src/libxl/libxl_domain.h| 8 ++-- src/libxl/libxl_driver.c| 81 - src/libxl/libxl_migration.c | 4 +- 4 files changed, 118 insertions(+), 81 deletions(-) -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 3/3] libxl: drop virDomainObj lock when destroying a domain
A destroy operation can take considerable time on large memory
domains due to scrubbing the domain's memory. Unlock the
virDomainObj while libxl_domain_destroy is executing.
Implement libxlDomainDestroyInternal wrapper to handle unlocking,
calling destroy, and locking. Change all callers of
libxl_domain_destroy to use the wrapper.
Signed-off-by: Jim Fehlig
---
V3:
Provide a wrapper to unlock, destroy and lock domain instead of
do so at each call site of libxl_domain_destroy.
src/libxl/libxl_domain.c| 29 ++---
src/libxl/libxl_domain.h| 4
src/libxl/libxl_driver.c| 6 +++---
src/libxl/libxl_migration.c | 4 ++--
4 files changed, 35 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 37ed082..1b809ca 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -435,7 +435,7 @@ libxlDomainShutdownThread(void *opaque)
libxlDomainEventQueue(driver, dom_event);
dom_event = NULL;
}
-libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
+libxlDomainDestroyInternal(driver, vm);
libxlDomainCleanup(driver, vm, reason);
if (!vm->persistent)
virDomainObjListRemove(driver->domains, vm);
@@ -447,7 +447,7 @@ libxlDomainShutdownThread(void *opaque)
libxlDomainEventQueue(driver, dom_event);
dom_event = NULL;
}
-libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
+libxlDomainDestroyInternal(driver, vm);
libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
if (libxlDomainStart(driver, vm, false, -1) < 0) {
virErrorPtr err = virGetLastError();
@@ -626,6 +626,29 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
}
/*
+ * Internal domain destroy function.
+ *
+ * virDomainObjPtr must be locked on invocation
+ */
+int
+libxlDomainDestroyInternal(libxlDriverPrivatePtr driver,
+ virDomainObjPtr vm)
+{
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+int ret = -1;
+
+/* Unlock virDomainObj during destroy, which can take considerable
+ * time on large memory domains.
+ */
+virObjectUnlock(vm);
+ret = libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
+virObjectLock(vm);
+
+virObjectUnref(cfg);
+return ret;
+}
+
+/*
* Cleanup function for domain that has reached shutoff state.
*
* virDomainObjPtr must be locked on invocation
@@ -1022,7 +1045,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
libxl_evdisable_domain_death(cfg->ctx, priv->deathW);
priv->deathW = NULL;
}
-libxl_domain_destroy(cfg->ctx, domid, NULL);
+libxlDomainDestroyInternal(driver, vm);
vm->def->id = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 30855a2..aa647b8 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -103,6 +103,10 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
libxlSavefileHeaderPtr ret_hdr)
ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
+int
+libxlDomainDestroyInternal(libxlDriverPrivatePtr driver,
+ virDomainObjPtr vm);
+
void
libxlDomainCleanup(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c9623ef..9eb071e 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1252,7 +1252,7 @@ libxlDomainDestroyFlags(virDomainPtr dom,
event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
-if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) {
+if (libxlDomainDestroyInternal(driver, vm) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to destroy domain '%d'"), vm->def->id);
goto endjob;
@@ -1595,7 +1595,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SAVED);
-if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) {
+if (libxlDomainDestroyInternal(driver, vm) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to destroy domain '%d'"), vm->def->id);
goto cleanup;
@@ -1802,7 +1802,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to,
unsigned int flags)
}
if (flags & VIR_DUMP_CRASH) {
-if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) {
+if (libxlDomainDestroyInternal(driver, vm) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to destroy domain '%d'"), vm->def->id);
goto unpause;
diff --git a/src/libxl/libxl_migration
[libvirt] [PATCH V3 2/3] libxl: acquire a job when destroying a domain
A job should be acquired at the beginning of a domain destroy operation,
not at the end when cleaning up the domain. Fix two occurrences of this
late job acquisition in the libxl driver. Doing so renders
libxlDomainCleanupJob unused, so it is removed.
Signed-off-by: Jim Fehlig
---
Unchanged from V2.
src/libxl/libxl_domain.c | 53
src/libxl/libxl_domain.h | 4
src/libxl/libxl_driver.c | 20 ++
3 files changed, 29 insertions(+), 48 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 391291d..37ed082 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -371,6 +371,9 @@ libxlDomainShutdownThread(void *opaque)
cfg = libxlDriverConfigGet(driver);
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
+goto cleanup;
+
if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
@@ -384,7 +387,7 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_PRESERVE:
case VIR_DOMAIN_LIFECYCLE_LAST:
-goto cleanup;
+goto endjob;
}
} else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
@@ -399,7 +402,7 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
case VIR_DOMAIN_LIFECYCLE_CRASH_LAST:
-goto cleanup;
+goto endjob;
case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY:
libxlDomainAutoCoreDump(driver, vm);
goto destroy;
@@ -420,11 +423,11 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_PRESERVE:
case VIR_DOMAIN_LIFECYCLE_LAST:
-goto cleanup;
+goto endjob;
}
} else {
VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
-goto cleanup;
+goto endjob;
}
destroy:
@@ -433,13 +436,11 @@ libxlDomainShutdownThread(void *opaque)
dom_event = NULL;
}
libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
-if (libxlDomainCleanupJob(driver, vm, reason)) {
-if (!vm->persistent) {
-virDomainObjListRemove(driver->domains, vm);
-vm = NULL;
-}
-}
-goto cleanup;
+libxlDomainCleanup(driver, vm, reason);
+if (!vm->persistent)
+virDomainObjListRemove(driver->domains, vm);
+
+goto endjob;
restart:
if (dom_event) {
@@ -447,13 +448,17 @@ libxlDomainShutdownThread(void *opaque)
dom_event = NULL;
}
libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
-libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
if (libxlDomainStart(driver, vm, false, -1) < 0) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to restart VM '%s': %s"),
vm->def->name, err ? err->message : _("unknown error"));
}
+ endjob:
+if (!libxlDomainObjEndJob(driver, vm))
+vm = NULL;
+
cleanup:
if (vm)
virObjectUnlock(vm);
@@ -691,26 +696,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
}
/*
- * Cleanup function for domain that has reached shutoff state.
- * Executed in the context of a job.
- *
- * virDomainObjPtr should be locked on invocation
- * Returns true if references remain on virDomainObjPtr, false otherwise.
- */
-bool
-libxlDomainCleanupJob(libxlDriverPrivatePtr driver,
- virDomainObjPtr vm,
- virDomainShutoffReason reason)
-{
-if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_DESTROY) < 0)
-return true;
-
-libxlDomainCleanup(driver, vm, reason);
-
-return libxlDomainObjEndJob(driver, vm);
-}
-
-/*
* Core dump domain to default dump path.
*
* virDomainObjPtr must be locked on invocation
@@ -735,15 +720,11 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver,
timestr) < 0)
goto cleanup;
-if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
-goto cleanup;
-
/* Unlock virDomainObj while dumping core */
virObjectUnlock(vm);
libxl_domain_core_dump(cfg->ctx, vm->def->id, dumpfile, NULL);
virObjectLock(vm);
-ignore_value(libxlDomainObjEndJob(driver, vm));
ret = 0;
cleanup:
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index a032e46..30855a2 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -108,10 +108,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
virDomainShutoffReason reason);
-bool
-libxlDomainCleanupJob(libxlDriverPrivatePtr driver,
-
[libvirt] [PATCH V3 1/3] libxl: Move job acquisition in libxlDomainStart to callers
Let callers of libxlDomainStart decide when it is appropriate to
acquire a job on the associated virDomainObj.
Signed-off-by: Jim Fehlig
---
Job handling in the migration code is currently broken/incomplete,
so fixing it is deferred to a follow-up series I'm working on.
src/libxl/libxl_domain.c | 24 -
src/libxl/libxl_driver.c | 55 +++-
2 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 37b2b58..391291d 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -903,16 +903,13 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
libxl_domain_config_init(&d_config);
-if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
-return ret;
-
cfg = libxlDriverConfigGet(driver);
/* If there is a managed saved state restore it instead of starting
* from scratch. The old state is removed once the restoring succeeded. */
if (restore_fd < 0) {
managed_save_path = libxlDomainManagedSavePath(driver, vm);
if (managed_save_path == NULL)
-goto endjob;
+goto cleanup;
if (virFileExists(managed_save_path)) {
@@ -920,7 +917,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
managed_save_path,
&def, &hdr);
if (managed_save_fd < 0)
-goto endjob;
+goto cleanup;
restore_fd = managed_save_fd;
@@ -934,7 +931,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
_("cannot restore domain '%s' uuid %s from a
file"
" which belongs to domain '%s' uuid %s"),
vm->def->name, vm_uuidstr, def->name,
def_uuidstr);
-goto endjob;
+goto cleanup;
}
virDomainObjAssignDef(vm, def, true, NULL);
@@ -951,14 +948,14 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def,
cfg->ctx, &d_config) < 0)
-goto endjob;
+goto cleanup;
if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0)
-goto endjob;
+goto cleanup;
if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
vm->def, VIR_HOSTDEV_SP_PCI) < 0)
-goto endjob;
+goto cleanup;
/* Unlock virDomainObj while creating the domain */
virObjectUnlock(vm);
@@ -990,7 +987,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to restore domain '%s'"),
d_config.c_info.name);
-goto endjob;
+goto cleanup;
}
/*
@@ -1037,7 +1034,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
libxlDomainEventQueue(driver, event);
ret = 0;
-goto endjob;
+goto cleanup;
cleanup_dom:
if (priv->deathW) {
@@ -1048,10 +1045,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
vm->def->id = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
- endjob:
-if (!libxlDomainObjEndJob(driver, vm))
-vm = NULL;
-
+ cleanup:
libxl_domain_config_dispose(&d_config);
VIR_FREE(dom_xml);
VIR_FREE(managed_save_path);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 05f6eb1..e315b32 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -303,18 +303,26 @@ libxlAutostartDomain(virDomainObjPtr vm,
virObjectLock(vm);
virResetLastError();
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
+virObjectUnlock(vm);
+return ret;
+}
+
if (vm->autostart && !virDomainObjIsActive(vm) &&
libxlDomainStart(driver, vm, false, -1) < 0) {
err = virGetLastError();
VIR_ERROR(_("Failed to autostart VM '%s': %s"),
vm->def->name,
err ? err->message : _("unknown error"));
-goto cleanup;
+goto endjob;
}
ret = 0;
- cleanup:
-virObjectUnlock(vm);
+
+ endjob:
+if (libxlDomainObjEndJob(driver, vm))
+virObjectUnlock(vm);
+
return ret;
}
@@ -885,17 +893,28 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
goto cleanup;
def = NULL;
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
+if (!vm->persistent) {
+virDomainObjListRemove(driver->domains, vm);
+vm = NULL;
+}
+goto cleanup;
+}
+
if
Re: [libvirt] [PATCH V2 0/3] libxl: domain destroy fixes
Jim Fehlig wrote: > V2 of a small series to fix issues wrt domain destroy > > https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html > > Comments from Konrad and Martin are addressed in this version. > > Jim Fehlig (3): > libxl: Move job acquisition in libxlDomainStart to callers > libxl: acquire a job when destroying a domain > libxl: drop virDomainObj lock when destroying a domain > While testing this series along with some ref counting improvements suggested by Martin, I noticed a few other places where libxl_domain_destroy is called with virDomainObj locked. I'm going to post a V3 that changes 3/3 to provide a wrapper for unlocking, destroying, and locking the domain. Existing libxl_domain_destroy callers are changed to use the wrapper. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RFC: whether or not to support multiple IDE controllers (and any IDE at all on Q35)
Due to a bug report by someone trying to add two cd drives to a Q35 domain in virt-manager (which assumes all CD drives are IDE), I've learned that libvirt doesn't support adding a "piix3-ide" controller when there is no ide controller for the domain (or when there are more than 4 disk devices that need to be connected to an IDE controller). It does dutifully add: as many times as necessary, it's just that there is no code behind that to add the device to the qemu commandline. I first thought I should add support for this, but then had second thoughts after following this reasoning: 1) I'm not sure exactly where the piix3-ide controller was added to qemu, but it was apparently sometime after qemu-1.2.0 (at least according to tests/qemuhelpdata/qemu-1.2.0-device). 2) At some point (again not sure, but it was there at least in qemu 1.0) support for a SATA controller was added (named ich9-achi by qemu), and it can also accept CD devices. So there is no version of qemu that has piix3-ide and no ich9-ahci. 3) "ide old, ahci new" 4) I'm doubtful there is any OS old enough to require an IDE disk controller rather than SATA that anyone would also want to put > 4 disk devices on. That leads me to think that maybe it is a better idea to just log an error if someone tries to add a disk with bus='ide' on a Q35 domain, or add more than 4 of them on an i440fx domain. Does anyone have an opinion, or (better yet) a smoking gun (e.g. problems with piix3-ide, or alternately problems with ahci or a situation where IDE would be required) that will force action in one direction or the other? (One possible example - although ahci was added on or before qemu 1.0, up until the most recent upstream it wasn't possible to migrate or save a domain with an ahci controller. Do we need to worry about people with qemu < 2.3.0 that want > 4 IDE devices? Or should we figure that if none have come up before now, likely none will come up in the future either?) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh.pod: Remove redundant --config from attach-interface
Signed-off-by: Jiri Denemark --- Pushed as trivial. tools/virsh.pod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 79d50f9..d642a69 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2458,7 +2458,7 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>. =item B I I I [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] -[I<--config>] [I<--inbound average,peak,burst>] [I<--outbound average,peak,burst>] +[I<--inbound average,peak,burst>] [I<--outbound average,peak,burst>] Attach a new network interface to the domain. I can be I to indicate connection via a libvirt virtual network, or -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Introduce virnetdevtest
This is yet another test for check of basic functionality of our
NIC state handling code.
Signed-off-by: Michal Privoznik
---
src/libvirt_private.syms | 1 +
src/util/virnetdev.c | 4 +-
src/util/virnetdev.h | 4 ++
tests/Makefile.am | 15 +
tests/virnetdevmock.c | 48 ++
tests/virnetdevtest.c | 94 +++
tests/virnetdevtestdata/eth0-broken/operstate | 1 +
tests/virnetdevtestdata/eth0-broken/speed | 1 +
tests/virnetdevtestdata/eth0/operstate| 1 +
tests/virnetdevtestdata/eth0/speed| 1 +
tests/virnetdevtestdata/lo/operstate | 1 +
tests/virnetdevtestdata/lo/speed | 1 +
12 files changed, 170 insertions(+), 2 deletions(-)
create mode 100644 tests/virnetdevmock.c
create mode 100644 tests/virnetdevtest.c
create mode 100644 tests/virnetdevtestdata/eth0-broken/operstate
create mode 100644 tests/virnetdevtestdata/eth0-broken/speed
create mode 100644 tests/virnetdevtestdata/eth0/operstate
create mode 100644 tests/virnetdevtestdata/eth0/speed
create mode 100644 tests/virnetdevtestdata/lo/operstate
create mode 100644 tests/virnetdevtestdata/lo/speed
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9f82926..0b42238 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1741,6 +1741,7 @@ virNetDevSetPromiscuous;
virNetDevSetRcvAllMulti;
virNetDevSetRcvMulti;
virNetDevSetupControl;
+virNetDevSysfsFile;
virNetDevValidateConfig;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 54d866e..a2d55a8 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1519,9 +1519,9 @@ int virNetDevValidateConfig(const char *ifname
ATTRIBUTE_UNUSED,
#ifdef __linux__
# define NET_SYSFS "/sys/class/net/"
-static int
+int
virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname,
- const char *file)
+ const char *file)
{
if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) < 0)
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 856127b..999a89a 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -219,4 +219,8 @@ int virNetDevSetRcvAllMulti(const char *ifname, bool
receive)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virNetDevGetRcvAllMulti(const char *ifname, bool *receive)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevSysfsFile(char **pf_sysfs_device_link,
+ const char *ifname,
+ const char *file)
+ATTRIBUTE_NONNULL(1);
#endif /* __VIR_NETDEV_H__ */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 046cd08..9ebedc3 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -176,6 +176,7 @@ test_programs = virshtest sockettest \
domainconftest \
virhostdevtest \
vircaps2xmltest \
+ virnetdevtest \
$(NULL)
if WITH_REMOTE
@@ -402,6 +403,7 @@ test_libraries = libshunload.la \
virnetserverclientmock.la \
vircgroupmock.la \
virpcimock.la \
+ virnetdevmock.la \
$(NULL)
if WITH_QEMU
test_libraries += libqemumonitortestutils.la \
@@ -1029,6 +1031,19 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \
virpcimock_la_LDFLAGS = -module -avoid-version \
-rpath /evil/libtool/hack/to/force/shared/lib/creation
+virnetdevtest_SOURCES = \
+ virnetdevtest.c testutils.h testutils.c
+virnetdevtest_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS)
+virnetdevtest_LDADD = $(LDADDS)
+
+virnetdevmock_la_SOURCES = \
+ virnetdevmock.c
+virnetdevmock_la_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS)
+virnetdevmock_la_LIBADD = $(GNULIB_LIBS) \
+ ../src/libvirt.la
+virnetdevmock_la_LDFLAGS = -module -avoid-version \
+-rpath /evil/libtool/hack/to/force/shared/lib/creation
+
if WITH_LINUX
virusbtest_SOURCES = \
virusbtest.c testutils.h testutils.c
diff --git a/tests/virnetdevmock.c b/tests/virnetdevmock.c
new file mode 100644
index 000..681e5fe
--- /dev/null
+++ b/tests/virnetdevmock.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * 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
Re: [libvirt] [PATCH] Fix xlconfigtest with older libxl
On Thu, Apr 02, 2015 at 09:34:25AM -0600, Jim Fehlig wrote: > Ján Tomko wrote: > > Commit cd5dc30 added this test, but it fails if > > LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is not defined: > > > > Opps, sorry. I tested back to Xen 4.3 where I thought this was > undefined. But looking at that machine now, I see > LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is defined. I guess your libxl is > from Xen 4.2? > It was on a Fedora 19 machine: xen-devel-4.2.4-4.fc19.x86_64 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] libvirt: virsh: Kill all uses of __FUNCTION__ in error messages
On 03.04.2015 12:33, Noella Ashu wrote:
> The error output of snapshot-revert should be more friendly. There is no
> need to show up virDomainRevertToSnapshot to user. virError already includes
> __FUNCTION__ information in a separate member of the struct, so repeating
> it in the message is redundant and leads to situations where higher level
> code ends up reporting the lower level name We correctly converted the
> error output making it more succinct and user-friendly.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1086726
> ---
> src/libvirt-domain-snapshot.c | 30 +++
> src/libvirt-domain.c | 201
> ++
> 2 files changed, 96 insertions(+), 135 deletions(-)
I guess I should have been more accurate: you need to make sure the code
compiles after your patch too. 'make syntax-check' and 'make check' are needed
too, but make no sense without pure 'make all'. What's needed is something like
following:
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f9db3ad..0acfd13 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1438,7 +1438,7 @@ virDomainScreenshot(virDomainPtr domain,
virCheckReadOnlyGoto(domain->conn->flags, error);
if (domain->conn != stream->conn) {
-virReportInvalidArg(stream, "%s",
+virReportInvalidArg(stream,
_("stream must match connection of domain '%s'"),
domain->name);
goto error;
@@ -2177,7 +2177,7 @@ virDomainGetMemoryParameters(virDomainPtr domain,
/* At most one of these two flags should be set. */
if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
(flags & VIR_DOMAIN_AFFECT_CONFIG)) {
-virReportInvalidArg(flags, "%s",
+virReportInvalidArg(flags, "%s",
_("flags 'affect live' and 'affect config' are
mutually exclusive"));
goto error;
}
Anyway, I've fixed all the compilation errors, ACKed and pushed.
Congratulations on your first libvirt contribution!
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt-1.2.14 candidate release 2
On Fri, Apr 03, 2015 at 08:25:56AM +0800, Daniel Veillard wrote: > On Thu, Apr 02, 2015 at 06:51:35PM +0200, Guido Günther wrote: > > Hi, > > On Tue, Mar 31, 2015 at 09:01:33AM +0800, Daniel Veillard wrote: > > >So I have just tagged in git and pushed the 1.2.14 candidate release > > > 2, it is available as usual as signed tarballs and rpms from: > > > > > > ftp://libvirt.org/libvirt/ > > > > > > I did push the 3 patches from Peter because what is the point of a > > > candidate release if it doesn't include the potentially controversial > > > bits that we intend for the final release ? That doesn't prevent someone > > > else than Eric to give the 2nd ACK for patch 3 of the series :-) > > > > > > So please git it a try, if needed we will just do an rc3 ! > > > BTW is there regression testing done from oVirt using libvirt > > > upstream on an automated basis ? If not that's something I > > > could help pushing for assuming we have public reg tests for oVirt. > > > > > > At least it seems to work in my minimal own tests, but others > > > should check, > > > > Looks good on Debian's Buildds: > > > > > > https://buildd.debian.org/status/package.php?p=libvirt&suite=experimental > > thanks for the feedback, but release is out, might want to push > the 1.2.14 final not rc2 :) Sure. It just takes a little longer here... -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState
> > ACK with the slight adjustments. > > If you want to post a v3 "inline"/as a response to patch 4 - that's fine > > John > Thank you, I sent v3 of patch 4/6, fixed the slight adjustments in 5 and 6. I'll push them once I have an ACK for 4/6. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] storage: Add support for storage pool state XML
This patch introduces new virStorageDriverState element stateDir.
Also adds necessary changes to storageStateInitialize, so that
directories initialization becomes more generic.
---
src/conf/storage_conf.h | 1 +
src/storage/storage_driver.c | 108 +++
2 files changed, 79 insertions(+), 30 deletions(-)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index da378b7..8d43019 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -293,6 +293,7 @@ struct _virStorageDriverState {
char *configDir;
char *autostartDir;
+char *stateDir;
bool privileged;
};
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 64ea770..b350ee6 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -78,6 +78,7 @@ static void
storageDriverAutostart(void)
{
size_t i;
+char *stateFile = NULL;
virConnectPtr conn = NULL;
/* XXX Remove hardcoding of QEMU URI */
@@ -126,7 +127,11 @@ storageDriverAutostart(void)
if (started) {
virStoragePoolObjClearVols(pool);
-if (backend->refreshPool(conn, pool) < 0) {
+stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml");
+if (!stateFile ||
+virStoragePoolSaveState(stateFile, pool->def) < 0 ||
+backend->refreshPool(conn, pool) < 0) {
virErrorPtr err = virGetLastError();
if (backend->stopPool)
backend->stopPool(conn, pool);
@@ -136,7 +141,9 @@ storageDriverAutostart(void)
virStoragePoolObjUnlock(pool);
continue;
}
+
pool->active = 1;
+VIR_FREE(stateFile);
}
virStoragePoolObjUnlock(pool);
}
@@ -147,61 +154,67 @@ storageDriverAutostart(void)
/**
* virStorageStartup:
*
- * Initialization function for the QEmu daemon
+ * Initialization function for the Storage Driver
*/
static int
storageStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
{
-char *base = NULL;
+int ret = -1;
+char *configdir = NULL;
+char *rundir = NULL;
if (VIR_ALLOC(driver) < 0)
-return -1;
+return ret;
if (virMutexInit(&driver->lock) < 0) {
VIR_FREE(driver);
-return -1;
+return ret;
}
storageDriverLock();
if (privileged) {
-if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0)
+if (VIR_STRDUP(driver->configDir,
+ SYSCONFDIR "/libvirt/storage") < 0 ||
+VIR_STRDUP(driver->autostartDir,
+ SYSCONFDIR "/libvirt/storage/autostart") < 0 ||
+VIR_STRDUP(driver->stateDir,
+ LOCALSTATEDIR "/run/libvirt/storage") < 0)
goto error;
} else {
-base = virGetUserConfigDirectory();
-if (!base)
+configdir = virGetUserConfigDirectory();
+rundir = virGetUserRuntimeDirectory();
+if (!(configdir && rundir))
+goto error;
+
+if ((virAsprintf(&driver->configDir,
+"%s/storage", configdir) < 0) ||
+(virAsprintf(&driver->autostartDir,
+"%s/storage", configdir) < 0) ||
+(virAsprintf(&driver->stateDir,
+ "%s/storage/run", rundir) < 0))
goto error;
}
driver->privileged = privileged;
-/*
- * Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/...
- * (session) or /etc/libvirt/storage/... (system).
- */
-if (virAsprintf(&driver->configDir,
-"%s/storage", base) == -1)
-goto error;
-
-if (virAsprintf(&driver->autostartDir,
-"%s/storage/autostart", base) == -1)
-goto error;
-
-VIR_FREE(base);
-
if (virStoragePoolLoadAllConfigs(&driver->pools,
driver->configDir,
driver->autostartDir) < 0)
goto error;
storageDriverUnlock();
-return 0;
+
+ret = 0;
+ cleanup:
+VIR_FREE(configdir);
+VIR_FREE(rundir);
+return ret;
error:
-VIR_FREE(base);
storageDriverUnlock();
storageStateCleanup();
-return -1;
+goto cleanup;
}
/**
@@ -261,6 +274,7 @@ storageStateCleanup(void)
VIR_FREE(driver->configDir);
VIR_FREE(driver->autostartDir);
+VIR_FREE(driver->stateDir);
storageDriverUnlock();
virMutexDestroy(&driver->lock);
VIR_FREE(driver);
@@ -579,6 +593,7 @@ storagePoolCreateXML(virConnectPtr conn,
virStoragePoolObjPtr pool = NULL;
virStoragePoolPtr ret = NULL;
virStorageBackendPtr backend;
+char *stateFile;
virCheckFlags(0, NULL);
@@ -6
[libvirt] [PATCH] libvirt: virsh: Kill all uses of __FUNCTION__ in error messages
The error output of snapshot-revert should be more friendly. There is no
need to show up virDomainRevertToSnapshot to user. virError already includes
__FUNCTION__ information in a separate member of the struct, so repeating
it in the message is redundant and leads to situations where higher level
code ends up reporting the lower level name We correctly converted the
error output making it more succinct and user-friendly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1086726
---
src/libvirt-domain-snapshot.c | 30 +++
src/libvirt-domain.c | 201 ++
2 files changed, 96 insertions(+), 135 deletions(-)
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 9feb669..ac858ba 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -222,26 +222,20 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) &&
!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) {
-virReportInvalidArg(flags,
-_("use of 'current' flag in %s requires "
- "'redefine' flag"),
-__FUNCTION__);
+virReportInvalidArg(flags, "%s",
+_("use of 'current' flag in requires 'redefine'
flag"));
goto error;
}
if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
-virReportInvalidArg(flags,
-_("'redefine' and 'no metadata' flags in %s are "
- "mutually exclusive"),
-__FUNCTION__);
+virReportInvalidArg(flags, "%s",
+_("'redefine' and 'no metadata' flags in are
mutually exclusive"));
goto error;
}
if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
(flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
-virReportInvalidArg(flags,
-_("'redefine' and 'halt' flags in %s are mutually "
- "exclusive"),
-__FUNCTION__);
+virReportInvalidArg(flags, "%s",
+_("'redefine' and 'halt' flags are mutually
exclusive"));
goto error;
}
@@ -1084,10 +1078,8 @@ virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if ((flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
-virReportInvalidArg(flags,
-_("running and paused flags in %s are mutually "
- "exclusive"),
-__FUNCTION__);
+virReportInvalidArg(flags, "%s",
+_("running and paused flags are mutually
exclusive"));
goto error;
}
@@ -1146,10 +1138,8 @@ virDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
if ((flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) &&
(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
-virReportInvalidArg(flags,
-_("children and children_only flags in %s are "
- "mutually exclusive"),
-__FUNCTION__);
+virReportInvalidArg(flags, "%s",
+_("children and children_only flags are mutually
exclusive"));
goto error;
}
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 8b5f91e..e0be331 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -385,9 +385,7 @@ virDomainLookupByUUIDString(virConnectPtr conn, const char
*uuidstr)
virCheckNonNullArgGoto(uuidstr, error);
if (virUUIDParse(uuidstr, uuid) < 0) {
-virReportInvalidArg(uuidstr,
-_("uuidstr in %s must be a valid UUID"),
-__FUNCTION__);
+virReportInvalidArg(uuidstr, "%s", _("Invalid UUID"));
goto error;
}
@@ -1440,9 +1438,9 @@ virDomainScreenshot(virDomainPtr domain,
virCheckReadOnlyGoto(domain->conn->flags, error);
if (domain->conn != stream->conn) {
-virReportInvalidArg(stream,
-_("stream in %s must match connection of domain
'%s'"),
-__FUNCTION__, domain->name);
+virReportInvalidArg(stream, "%s",
+_("stream must match connection of domain '%s'"),
+domain->name);
goto error;
}
@@ -2179,10 +2177,8 @@ virDomainGetMemoryParameters(virDomainPtr domain,
/* At most one of these two flags should be set. */
if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
(flags & VIR_DOMAIN_AFFECT_CONFIG)) {
-virReportInvalidArg(flags,
-_("flags 'affect live' and 'affect config' in %s "
- "are mutually ex
Re: [libvirt] [PATCH] libvirt: virsh: Kill all uses of __FUNCTION__ in error messages
Hello +Michal, I have sent a new patch to the mailing list with the whitespaces removed. This should work fine now. Please just let me know if you have any other issues. Thanks, Noella On Fri, Apr 3, 2015 at 11:04 AM, Noella Ashu wrote: > Hello +Michal, > > I will make the corrections and re send the patch. Sorry for the errors. > > Thanks, > Noella > > On Fri, Apr 3, 2015 at 10:19 AM, Michal Privoznik > wrote: > >> On 01.04.2015 14:46, Noella Ashu wrote: >> > The error output of snapshot-revert should be more friendly. There is no >> > need to show up virDomainRevertToSnapshot to user. virError already >> includes >> > __FUNCTION__ information in a separate member of the struct, so >> repeating >> > it in the message is redundant and leads to situations where higher >> level >> > code ends up reporting the lower level name We correctly converted the >> > error output making it more succinct and user-friendly. >> > >> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1086726 >> > --- >> > src/libvirt-domain-snapshot.c | 30 +++ >> > src/libvirt-domain.c | 201 >> ++ >> > 2 files changed, 96 insertions(+), 135 deletions(-) >> >> I'm having some difficulties applying this patch: >> >> Applying: libvirt: virsh: Kill all uses of __FUNCTION__ in error messages >> /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:91: trailing >> whitespace. >> virReportInvalidArg(stream, "%s", >> /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:105: >> trailing whitespace. >> virReportInvalidArg(flags, "%s", >> /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:118: >> trailing whitespace. >> virReportInvalidArg(flags, "%s", >> /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:233: >> trailing whitespace. >> virReportInvalidArg(conn, "%s", >> /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:309: >> trailing whitespace. >> virReportInvalidArg(nkeycodes, "%s", >> error: patch failed: src/libvirt-domain.c:11284 >> error: src/libvirt-domain.c: patch does not apply >> Patch failed at 0001 libvirt: virsh: Kill all uses of __FUNCTION__ in >> error messages >> The copy of the patch that failed is found in: >>/home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch >> When you have resolved this problem, run "git am --continue". >> If you prefer to skip this patch, run "git am --skip" instead. >> To restore the original branch and stop patching, run "git am --abort". >> >> >> Can you please rebase and resend? Oh, and don't forget to run 'make >> syntax-check check' before sending a patch. >> >> Michal >> > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix domifaddr no output in quiet mode
On 03.04.2015 11:41, Luyao Huang wrote: > All of print function use vshPrintExtra in cmdDomIfAddr, this will > make domifaddr no output in quiet mode, after this patch, quiet mode > output will be: > > # virsh -q domifaddr test3 --source agent > lo 00:00:00:00:00:00ipv4 127.0.0.1/8 > - -ipv6 ::1/128 > ens8 52:54:00:1a:cb:3fipv6 fe80::5054:ff:fe1a:cb3f/64 > virbr0 52:54:00:db:51:e7ipv4 192.168.122.1/24 > virbr0-nic 52:54:00:db:51:e7N/A N/A > > > Signed-off-by: Luyao Huang > --- > tools/virsh-domain-monitor.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > I've reworded the commit message, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: fix 2 issue around cpuset
There are two bugs in this function:
1. cannot start a vm with cpuset but without numatune settings
# virsh -c lxc:/// start helloworld
error: Failed to start domain helloworld
error: internal error: guest failed to start: Invalid value '1-3' for
'cpuset.mems': Invalid argument
we don't free &mask after use it for virCgroupSetCpusetCpus() and
then virDomainNumatuneMaybeFormatNodeset() do not get a new &mask,
then we use it in virCgroupSetCpusetMems().
2. when start a lxc with numatune memory mode not strict
# virsh -c lxc:/// start helloworld
error: Failed to start domain helloworld
error: internal error: guest failed to start: Unknown failure in libvirt_lxc
startup
We shouldn't set anything in cpuset.mems for these mode.
Signed-off-by: Luyao Huang
---
src/lxc/lxc_cgroup.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index c1813e2..5e959a2 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -77,11 +77,15 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
if (virCgroupSetCpusetCpus(cgroup, mask) < 0)
goto cleanup;
+/* free mask to make sure we won't use it in a wrong way later */
+VIR_FREE(mask);
}
if (virDomainNumatuneGetMode(def->numa, -1) !=
-VIR_DOMAIN_NUMATUNE_MEM_STRICT)
+VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+ret = 0;
goto cleanup;
+}
if (virDomainNumatuneMaybeFormatNodeset(def->numa, nodemask,
&mask, -1) < 0)
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt: virsh: Kill all uses of __FUNCTION__ in error messages
Hello +Michal, I will make the corrections and re send the patch. Sorry for the errors. Thanks, Noella On Fri, Apr 3, 2015 at 10:19 AM, Michal Privoznik wrote: > On 01.04.2015 14:46, Noella Ashu wrote: > > The error output of snapshot-revert should be more friendly. There is no > > need to show up virDomainRevertToSnapshot to user. virError already > includes > > __FUNCTION__ information in a separate member of the struct, so > repeating > > it in the message is redundant and leads to situations where higher level > > code ends up reporting the lower level name We correctly converted the > > error output making it more succinct and user-friendly. > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1086726 > > --- > > src/libvirt-domain-snapshot.c | 30 +++ > > src/libvirt-domain.c | 201 > ++ > > 2 files changed, 96 insertions(+), 135 deletions(-) > > I'm having some difficulties applying this patch: > > Applying: libvirt: virsh: Kill all uses of __FUNCTION__ in error messages > /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:91: trailing > whitespace. > virReportInvalidArg(stream, "%s", > /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:105: trailing > whitespace. > virReportInvalidArg(flags, "%s", > /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:118: trailing > whitespace. > virReportInvalidArg(flags, "%s", > /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:233: trailing > whitespace. > virReportInvalidArg(conn, "%s", > /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:309: trailing > whitespace. > virReportInvalidArg(nkeycodes, "%s", > error: patch failed: src/libvirt-domain.c:11284 > error: src/libvirt-domain.c: patch does not apply > Patch failed at 0001 libvirt: virsh: Kill all uses of __FUNCTION__ in > error messages > The copy of the patch that failed is found in: >/home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". > > > Can you please rebase and resend? Oh, and don't forget to run 'make > syntax-check check' before sending a patch. > > Michal > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] esx: Make esxNodeGetFreeMemory always use ESX host.
On 09.03.2015 16:54, Dawid Zamirski wrote: > This patch is intended to make ESX driver behave more consistently. > When calling virNodeGetFreeMemory, it would return the free bytes > from a cluster when using vpx connection or ESX host when using esx > connection. However, virGetNodeInfo always returns info from ESX > host (specifying ESX host is mandatory, even for vpx connections) > so to make this consitent, these patches make virNodeGetFreeMemory > always return info from the ESX host. > > Dawid Zamirski (2): > esx: add esxVI_GetInt > esx: esxNodeGetFreeMemory return info from host. > > src/esx/esx_driver.c | 53 > ++ > src/esx/esx_vi.c | 32 ++ > src/esx/esx_vi.h | 3 +++ > src/esx/esx_vi_types.c | 3 +++ > src/esx/esx_vi_types.h | 1 + > 5 files changed, 58 insertions(+), 34 deletions(-) > ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] qemu: migration: continuously sending and receiving ARP packets guest mistakenly thinks there's another guest with the same IP.
Problem Description: Live-migrate a guest, which has a tap device and continuously sends and receives ARP packets, it would mistakenly think there's another guest with the same IP, immedially after migration. The steps to reproduce the problem: 1 define and start a domain with its network configured as: 2 The guest sends ARP packets continuously: arping -I ethX xx.xx.xx.xx(self_ip) 3 Meanwhile, the guest also receives ARP packets continuously: tcpdump -i ethX arp host xx.xx.xx.xx(self_ip) -en 4 After migrateion, at the dest side, the guest gets a lot of ARP packets which came from the source-side guest(which was stored while it's suspended.). For example: 2015-03-27 16:45:56.695166 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695197 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695205 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695214 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695244 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695256 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695264 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695291 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695324 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695337 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695344 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695364 52:54:00:7d:b0:af > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 5 These packets will confuse my process. It may think that there is another VM has the same IP with itself. Reasons for the problem: The tap device will get up as soon as it's created(in virNetDevTapCreateInBridgePort), before the cpus got un-paused. So, it kept receiving data before the guest starts to run, please note that the data are sent from the source side. As soon as the guest get running, it parses the data stored before, and thinks they were from other guest with the same IP, which is in fact the guest from the source side. There was a patch "network: Bring netdevs online later", it move the virNetDevSetOnline() of network device just before start VM's vcpu. But in the Laine Stump replay mail say "It turns out, though, that regular tap devices which will be connected to a bridge should be ifup'ed and attached to the bridge as soon as possible, so that the forwarding delay timer of the bridge can start to count down." I agree with Laine Stump's idea that it's not a perfect solution to start the tap device right before running vcpu. so, here comes the question: what can we do to insure our guests not receive itself's ARP packets from src side during migrateion? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix domifaddr no output in quiet mode
All of print function use vshPrintExtra in cmdDomIfAddr, this will
make domifaddr no output in quiet mode, after this patch, quiet mode
output will be:
# virsh -q domifaddr test3 --source agent
lo 00:00:00:00:00:00ipv4 127.0.0.1/8
- -ipv6 ::1/128
ens8 52:54:00:1a:cb:3fipv6 fe80::5054:ff:fe1a:cb3f/64
virbr0 52:54:00:db:51:e7ipv4 192.168.122.1/24
virbr0-nic 52:54:00:db:51:e7N/A N/A
Signed-off-by: Luyao Huang
---
tools/virsh-domain-monitor.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 6951db2..9686531 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -2278,9 +2278,9 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd)
/* When the interface has no IP address */
if (!iface->naddrs) {
-vshPrintExtra(ctl, " %-10s %-17s%-12s %s\n",
- iface->name,
- iface->hwaddr ? iface->hwaddr : "N/A", "N/A", "N/A");
+vshPrint(ctl, " %-10s %-17s%-12s %s\n",
+ iface->name,
+ iface->hwaddr ? iface->hwaddr : "N/A", "N/A", "N/A");
continue;
}
@@ -2313,12 +2313,12 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd)
/* Don't repeat interface name */
if (full || !j)
-vshPrintExtra(ctl, " %-10s %-17s%s\n",
- iface->name,
- iface->hwaddr ? iface->hwaddr : "", ip_addr_str);
+vshPrint(ctl, " %-10s %-17s%s\n",
+ iface->name,
+ iface->hwaddr ? iface->hwaddr : "", ip_addr_str);
else
-vshPrintExtra(ctl, " %-10s %-17s%s\n",
- "-", "-", ip_addr_str);
+vshPrint(ctl, " %-10s %-17s%s\n",
+ "-", "-", ip_addr_str);
virBufferFreeAndReset(&buf);
VIR_FREE(ip_addr_str);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt: virsh: Kill all uses of __FUNCTION__ in error messages
On 01.04.2015 14:46, Noella Ashu wrote: > The error output of snapshot-revert should be more friendly. There is no > need to show up virDomainRevertToSnapshot to user. virError already includes > __FUNCTION__ information in a separate member of the struct, so repeating > it in the message is redundant and leads to situations where higher level > code ends up reporting the lower level name We correctly converted the > error output making it more succinct and user-friendly. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1086726 > --- > src/libvirt-domain-snapshot.c | 30 +++ > src/libvirt-domain.c | 201 > ++ > 2 files changed, 96 insertions(+), 135 deletions(-) I'm having some difficulties applying this patch: Applying: libvirt: virsh: Kill all uses of __FUNCTION__ in error messages /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:91: trailing whitespace. virReportInvalidArg(stream, "%s", /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:105: trailing whitespace. virReportInvalidArg(flags, "%s", /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:118: trailing whitespace. virReportInvalidArg(flags, "%s", /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:233: trailing whitespace. virReportInvalidArg(conn, "%s", /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch:309: trailing whitespace. virReportInvalidArg(nkeycodes, "%s", error: patch failed: src/libvirt-domain.c:11284 error: src/libvirt-domain.c: patch does not apply Patch failed at 0001 libvirt: virsh: Kill all uses of __FUNCTION__ in error messages The copy of the patch that failed is found in: /home/zippy/work/libvirt/libvirt.git/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Can you please rebase and resend? Oh, and don't forget to run 'make syntax-check check' before sending a patch. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
> > This change from configDir > >> +int virStoragePoolSaveState(const char *stateFile, >> +virStoragePoolDefPtr def); >> +int virStoragePoolSaveConfig(const char *configFile, > > to configFile should be in a separate commit. > > (You can just push it under the trivial rule) > > Jan > Thanks for notice, I sent a patch to list and pushed it as trivial. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Change virStoragePoolSaveConfig prototype s/configDir/configFile
Just a minor change which might be a little confusing for someone looking only at the API. --- Pushed as trivial. src/conf/storage_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index edcdaed..da378b7 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -373,7 +373,7 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, int virStoragePoolSaveState(const char *stateFile, virStoragePoolDefPtr def); -int virStoragePoolSaveConfig(const char *configDir, +int virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def); int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver, virStoragePoolObjPtr pool, -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
> Your previous patch 3/7 had a better commit message: > > Introduce virStoragePoolSaveStatus to properly format the status XML in > the same manner as virStoragePoolDefFormat, except for adding a > ... around the definition. This is similar to > virNetworkObjFormat used to save the live/active network information. > >> >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 > > By itself - no it doesn't solve the bug (same for 4 and 5) >> --- >> src/conf/storage_conf.c | 35 +++ >> src/conf/storage_conf.h | 4 +++- >> src/libvirt_private.syms | 1 + >> 3 files changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c >> index 73b937e..ee564f2 100644 >> --- a/src/conf/storage_conf.c >> +++ b/src/conf/storage_conf.c >> @@ -1928,6 +1928,41 @@ static int virStoragePoolSaveXML(const char *path, >> >> return ret; >> } >> + >> + >> +int virStoragePoolSaveState(const char *stateFile, >> +virStoragePoolDefPtr def) > > Again it's > int > virStorage... > > > ACK with those adjustments. > > John > > FYI: Coverity is happy with all 6 patches... Thank you, I changed the commit message, fixed the issues and pushed with Jan's little note in mind (I'll push that one as trivial). Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] conf: Introduce virStoragePoolDefFormatBuf
> > ACK with one nit: > >> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c >> index b070448..a8e9876 100644 >> --- a/src/conf/storage_conf.c >> +++ b/src/conf/storage_conf.c >> @@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf, > > --- 8< --- > >> -virBufferAdjustIndent(&buf, -2); >> -virBufferAddLit(&buf, "\n"); >> +virBufferAdjustIndent(buf, -2); >> +virBufferAddLit(buf, "\n"); >> + >> +return 0; >> + >> + error: >> +return -1; > > The error label is not necessary - just return -1; directly (unless you > plan to add some cleanup code there in the near future). > > 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/6] conf: Introduce virStoragePoolSaveState
On Thu, Apr 02, 2015 at 12:10:37PM +0200, Erik Skultety wrote: > Properly format storage pool state XML. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 > --- > src/conf/storage_conf.c | 35 +++ > src/conf/storage_conf.h | 4 +++- > src/libvirt_private.syms | 1 + > 3 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h > index 4584075..da378b7 100644 > --- a/src/conf/storage_conf.h > +++ b/src/conf/storage_conf.h > @@ -371,7 +371,9 @@ virStoragePoolObjPtr > virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, > virStoragePoolDefPtr def); > > -int virStoragePoolSaveConfig(const char *configDir, This change from configDir > +int virStoragePoolSaveState(const char *stateFile, > +virStoragePoolDefPtr def); > +int virStoragePoolSaveConfig(const char *configFile, to configFile should be in a separate commit. (You can just push it under the trivial rule) 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 1/6] conf: Introduce virStoragePoolDefFormatBuf
On Thu, Apr 02, 2015 at 12:10:35PM +0200, Erik Skultety wrote: > When modifying config/status XML, it might be handy to include some > additional XML elements (e.g. ). In order to do so, > introduce new formatting function virStoragePoolDefFormatBuf and make > virStoragePoolDefFormat call it. > --- > src/conf/storage_conf.c | 78 > + > 1 file changed, 46 insertions(+), 32 deletions(-) > ACK with one nit: > diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c > index b070448..a8e9876 100644 > --- a/src/conf/storage_conf.c > +++ b/src/conf/storage_conf.c > @@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf, --- 8< --- > -virBufferAdjustIndent(&buf, -2); > -virBufferAddLit(&buf, "\n"); > +virBufferAdjustIndent(buf, -2); > +virBufferAddLit(buf, "\n"); > + > +return 0; > + > + error: > +return -1; The error label is not necessary - just return -1; directly (unless you plan to add some cleanup code there in the near future). 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 1/3] qemu: qemuDomainHotplugVcpus - separate out the add cgroup
On Thu, Apr 02, 2015 at 12:24:40PM -0400, John Ferlan wrote: > Future IOThread setting patches would copy the code anyway, so create > and generalize the add the vcpu to a cgroup into its own API. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_driver.c | 69 > +++--- > 1 file changed, 48 insertions(+), 21 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 6132674..fa880b7 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -4635,9 +4635,49 @@ static void qemuProcessEventHandler(void *data, void > *opaque) > VIR_FREE(processEvent); > } > > -static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, > - virDomainObjPtr vm, > - unsigned int nvcpus) > +typedef int cgroupNewFunc(virCgroupPtr domain, > + int id, > + bool create, > + virCgroupPtr *group); > + We have three virCgroupNew* functions that only differ by the parameters of the virAsprintf call. Would it make sense to turn them into one like this: virCgroupNewThread(virCgroupThreadName name, int id, ...) which would take the name of the thread from the enum, instead of the function name? Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
