[libvirt] qemu: lifecycle: reboot + shutdown, unexpected vm status.
Steps:
1 virsh reboot guest1 --mode=acpi
2 virsh shutdown guest1 --mode=agent
Expected result:
As the SHUTDOWN job is after REBOOT, we expected the guest to be *shutoff*.
(Do you think so?)
Exacted result:
After the 2 steps above, the guest got *rebooted*.
The reason to this problem:
1 in qemuDomainReboot(mode acpi), it sets priv-fakeReboot to 1.
2 after shutdown/reboot, qemu monitor IO trigged
qemuProcessHandleShutdown(), which finds that priv-fakeReboot is 1, and reboot
the guest.
Root Cause of the problem:
After further look into the problem, We found that the design of
acpi/agent shutdown/reboot seems a little chaotic.
---
sheet1 who sets fakeReboot
---
shutdown reboot
acpiY(0) Y(1)
agent N N
It's apparently, *acpi-mode* jobs set fakeReboot.
---
sheet2 who needs to check fakeReboot(qemuProcessHandleShutdown())
---
shutdown reboot
acpiY Y
agent *Y* *N*
Things become a little odd here. only agent-mode reboot doesn't check
fakeReboot.
We can tell from the above 2 sheets, that they're not consistent.
*Agent-mode shutdown needs to check fakeReboot(trigger by SHUTDOWN
monitorIO event), while it didn't set it before.*
The chaos is not caused by libvirtd, it's a systematic problem, including
guest os and qemu. (guest os writes ACPI, triggers qemu, qemu then triggers
libvirtd)
My Solution:
A simple solution is to make the 1st sheet consistent to the 2nd sheet,
which is:
---
sheet3 who should set fakeReboot:
---
shutdown reboot
acpiY(0) Y(1)
agent *Y(0)* N
---
we let agent-mode shutdown set fakeReboot to 0.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7eb5a7d..c751dcf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1959,6 +1959,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
goto endjob;
}
+qemuDomainSetFakeReboot(driver, vm, isReboot);
+
if (useAgent) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv-agent, agentFlag);
@@ -1970,7 +1972,6 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
*/
if (!useAgent ||
(ret 0 (acpiRequested || !flags))) {
-qemuDomainSetFakeReboot(driver, vm, isReboot);
/* Even if agent failed, we have to check if guest went away
* by itself while our locks were down. */
--
Discussion:
Although the solution above seems to have solved the problem, but it seems
a little awkward to have sheets like this.
Maybe we should let sheet2 consistent with sheet1, rather than letting
sheet1 consistent to sheet2. But It's too difficult to realize that, seems
impossible
So, is there a better way to solve this problem? or, shall I commit this
patch?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] util: client: shall libvirtd record task histories?
On 2015/4/10 15:54, Jiri Denemark wrote: On Wed, Apr 08, 2015 at 15:40:36 +0800, zhang bo wrote: We recently encountered a problem: 1) migrate a domain 2) the client unexpectedly got *crashed* (let's take it as virsh command) 3) *libvirtd still kept migrating the domain* 4) after it's restarted, the client didn't know the guest is still migrating. The problem is that libvirtd and the client has different view of the task state. After migration, the client may wrongly think that something's wrong that the domain got unexpectedly migrated. In my opinion, libvirtd should just *execute* tasks, like the hands of a human, while clients should be the brain to *schedule and remember* tasks. So, In order to avoid this problem,we should let the client record all the taskes somewhere, and reload the states after its restart. the client may cancel or continue the task as it wishes. Libvirtd should not record the task status. Not really. It's libvirtd, the daemon, which has to remember everything. It manages the state of all domains running on a host and synchronizes all clients that want to change state of the domains. Remember, even if a client is not restarted, domains my unexpectedly migrate somewhere else because another client might have asked for it. That said, if you're implementing a higher management layer which manages domains using libvirt and you know it is going to be the only client talking directly to libvirt, you can remember the state there if you want. However, it's not something libvirt itself should or could do. But you will most likely need to synchronize the state with libvirtd in case the client is restarted. Even libvirtd has to synchronize its internal state with all running QEMU processes when it is restarted because the state might have changed. Jirka . Thank you Jirka. Let's go a step further, suppose that the client doesn't crash at step 2), it's just disconnected to libvirtd at src side. 1) client(nova) calls virDomainMigrateToURI2() to migrate a guest 2) libvirtd at src side connects to libvirtd at dest side. 3) Unfortunately, somehow, client(nova) gets disconnected to libvirtd while migrating the guest. 4) the API virDomainMigrateToURI2() returns with error in client(nova) 5) but libvirtd doesn't aware that the connection to client is broken, and keeps migrating the guest to dest. 6) the guest is migrated to the dest side eventually. 7) Because the nova at src side thinks migration is not successed as step 4), the nova at the dest will consider the migrated-in guest as an unexpected running guest, and will shut it down. The guest disappears at last, due to the previous disconnection of libvirtd client and server. Even though libvirtd remembers everything, the client at dest side still wrongly killed the guest after migration. So, how to solve this problem? Shall libvirtd keep watching its clients' connection, and cancel running jobs concerning the disconnected client immediately after the client disconnects? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter: Partly initialize driver even for non-privileged users
https://bugzilla.redhat.com/show_bug.cgi?id=1211436
This reverts commit b7829f959b33c6e3242a9ed745c0da7dc696.
The previous fix was not correct. Like everywhere else, a driver is a
global variable allocated in stateInitialize function (or something
similar for stateless drivers). Later, when a driver API is called,
it's possible that the global variable is accessed and dereferenced.
Now, some drivers require root privileges because they undertake some
actions reserved only for the system admin (e.g. manipulating host
firewall). And here's the trouble, the NWFilter state initializer
exited too early when finding out it's running unprivileged, leaving
the global NWFilter driver variable uninitialized. Any subsequent
API call that tried to lock the driver resulted in dereferencing the
driver and thus crash.
On the other hand, in order to not resurrect the bug the original
commit was fixing, Let's forbid the nwfilter define in session mode.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Conflicts:
src/nwfilter/nwfilter_driver.c: Context. Code changed a bit
since 2013.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/nwfilter/nwfilter_driver.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 8e3db43..1a868b6 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -176,9 +176,6 @@ nwfilterStateInitialize(bool privileged,
char *base = NULL;
DBusConnection *sysbus = NULL;
-if (!privileged)
-return 0;
-
if (virDBusHasSystemBus()
!(sysbus = virDBusGetSystemBus()))
return -1;
@@ -193,6 +190,9 @@ nwfilterStateInitialize(bool privileged,
driver-watchingFirewallD = (sysbus != NULL);
driver-privileged = privileged;
+if (!privileged)
+return 0;
+
nwfilterDriverLock();
if (virNWFilterIPAddrMapInit() 0)
@@ -535,6 +535,12 @@ nwfilterDefineXML(virConnectPtr conn,
virNWFilterObjPtr nwfilter = NULL;
virNWFilterPtr ret = NULL;
+if (!driver-privileged) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Can't define NWFilters in session mode));
+return NULL;
+}
+
nwfilterDriverLock();
virNWFilterWriteLockFilterUpdates();
virNWFilterCallbackDriversLock();
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] qemuProcessStop: wake up pending sync block jobs
Other threads may be blocked in qemuBlockJobSyncWait. Ensure that
they're woken up when the domain is stopped.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/qemu/qemu_process.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 276837e..5390bc8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5086,6 +5086,13 @@ void qemuProcessStop(virQEMUDriverPtr driver,
if (virAtomicIntDecAndTest(driver-nactive) driver-inhibitCallback)
driver-inhibitCallback(false, driver-inhibitOpaque);
+/* Wake up anything waiting on synchronous block jobs */
+for (i = 0; i vm-def-ndisks; i++) {
+virDomainDiskDefPtr disk = vm-def-disks[i];
+if (disk-blockJobSync disk-blockJobStatus == -1)
+virCondSignal(disk-blockJobSyncCond);
+}
+
if ((logfile = qemuDomainCreateLog(driver, vm, true)) 0) {
/* To not break the normal domain shutdown process, skip the
* timestamp log writing if failed on opening log file. */
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] qemuBlockJobSync*: introduce sync block job helpers
qemuBlockJobSyncBegin and qemuBlockJobSyncEnd delimit a region of code
where block job events are processed synchronously.
qemuBlockJobSyncWait and qemuBlockJobSyncWaitWithTimeout wait for an
event generated by a block job.
The Wait* functions may be called multiple times while the synchronous
block job is active. Any pending block job event will be processed by
only when Wait* or End is called. disk-blockJobStatus is reset by
these functions, so if it is needed a pointer to a
virConnectDomainEventBlockJobStatus variable should be passed as the
last argument. It is safe to pass NULL if you do not care about the
block job status.
All functions assume the VM object is locked. The Wait* functions will
unlock the object for as long as they are waiting. They will return -1
and report an error if the domain exits before an event is received.
Typical use is as follows:
virQEMUDriverPtr driver;
virDomainObjPtr vm; /* locked */
virDomainDiskDefPtr disk;
virConnectDomainEventBlockJobStatus status;
qemuBlockJobSyncBegin(disk);
... start block job ...
if (qemuBlockJobSyncWait(driver, vm, disk, status) 0) {
/* domain died while waiting for event */
ret = -1;
goto error;
}
... possibly start other block jobs
or wait for further events ...
qemuBlockJobSyncEnd(driver, vm, disk, NULL);
To perform other tasks periodically while waiting for an event:
virQEMUDriverPtr driver;
virDomainObjPtr vm; /* locked */
virDomainDiskDefPtr disk;
virConnectDomainEventBlockJobStatus status;
unsigned long long timeout = 500 * 1000ull; /* milliseconds */
qemuBlockJobSyncBegin(disk);
... start block job ...
do {
... do other task ...
if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk,
timeout, status) 0) {
/* domain died while waiting for event */
ret = -1;
goto error;
}
} while (status == -1);
qemuBlockJobSyncEnd(driver, vm, disk, NULL);
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
po/POTFILES.in | 1 +
src/qemu/qemu_blockjob.c | 164 +++
src/qemu/qemu_blockjob.h | 16 +
3 files changed, 181 insertions(+)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index dd06ab3..bb0f6e1 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -112,6 +112,7 @@ src/parallels/parallels_utils.h
src/parallels/parallels_storage.c
src/phyp/phyp_driver.c
src/qemu/qemu_agent.c
+src/qemu/qemu_blockjob.c
src/qemu/qemu_capabilities.c
src/qemu/qemu_cgroup.c
src/qemu/qemu_command.c
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index e61ad8c..729928a 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -31,6 +31,8 @@
#include virlog.h
#include virstoragefile.h
+#include virthread.h
+#include virtime.h
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -165,3 +167,165 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
virObjectUnref(cfg);
}
+
+
+/**
+ * qemuBlockJobSyncBegin:
+ * @disk: domain disk
+ *
+ * Begin a new synchronous block job for @disk. The synchronous
+ * block job is ended by a call to qemuBlockJobSyncEnd, or by
+ * the guest quitting.
+ *
+ * During a synchronous block job, a block job event for @disk
+ * will not be processed asynchronously. Instead, it will be
+ * processed only when qemuBlockJobSyncWait* or
+ * qemuBlockJobSyncEnd is called.
+ */
+void
+qemuBlockJobSyncBegin(virDomainDiskDefPtr disk)
+{
+if (disk-blockJobSync)
+VIR_WARN(Disk %s already has synchronous block job,
+ disk-dst);
+
+disk-blockJobSync = true;
+}
+
+
+/**
+ * qemuBlockJobSyncEnd:
+ * @driver: qemu driver
+ * @vm: domain
+ * @disk: domain disk
+ * @ret_status: pointer to virConnectDomainEventBlockJobStatus
+ *
+ * End a synchronous block job for @disk. Any pending block job event
+ * for the disk is processed, and its status is recorded in the
+ * virConnectDomainEventBlockJobStatus field pointed to by
+ * @ret_status.
+ */
+void
+qemuBlockJobSyncEnd(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virDomainDiskDefPtr disk,
+virConnectDomainEventBlockJobStatus *ret_status)
+{
+if (disk-blockJobSync disk-blockJobStatus != -1) {
+if (ret_status)
+*ret_status = disk-blockJobStatus;
+qemuBlockJobEventProcess(driver, vm, disk,
+ disk-blockJobType,
+ disk-blockJobStatus);
+disk-blockJobStatus = -1;
+}
+disk-blockJobSync = false;
+}
+
+
+/**
+ * qemuBlockJobSyncWaitWithTimeout:
+ * @driver: qemu driver
+ * @vm: domain
+ * @disk: domain disk
+ * @timeout: timeout in milliseconds
+ * @ret_status: pointer to virConnectDomainEventBlockJobStatus
+ *
+ * Wait up to @timeout milliseconds for a block job event for @disk.
+ * If an event is received it is processed, and its status is recorded
+ * in
[libvirt] [PATCH 5/5] qemu: migration: use sync block job helpers
In qemuMigrationDriveMirror we can start all disk mirrors in parallel.
We wait until they are all ready, or one of them aborts.
In qemuMigrationCancelDriveMirror, we wait until all mirrors are
properly stopped. This is necessary to ensure that destination VM is
fully in sync with the (paused) source VM.
If a drive mirror can not be cancelled, then the destination is not in a
consistent state. In this case it is not safe to continue with the
migration.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/qemu/qemu_migration.c | 439 --
1 file changed, 266 insertions(+), 173 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 611f53a..752097a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -39,6 +39,7 @@
#include qemu_command.h
#include qemu_cgroup.h
#include qemu_hotplug.h
+#include qemu_blockjob.h
#include domain_audit.h
#include virlog.h
@@ -1679,6 +1680,200 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
goto cleanup;
}
+
+static int
+qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuMigrationCookiePtr mig)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+
+if (!mig-nbd)
+return 0;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm,
+ QEMU_ASYNC_JOB_MIGRATION_IN) 0)
+return -1;
+
+if (qemuMonitorNBDServerStop(priv-mon) 0)
+VIR_WARN(Unable to stop NBD server);
+if (qemuDomainObjExitMonitor(driver, vm) 0)
+return -1;
+
+virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
+priv-nbdPort = 0;
+return 0;
+}
+
+
+/**
+ * qemuMigrationCheckDriveMirror:
+ * @driver: qemu driver
+ * @vm: domain
+ *
+ * Check the status of all drive-mirrors started by
+ * qemuMigrationDriveMirror. Any pending block job events
+ * for the mirrored disks will be processed.
+ *
+ * Returns 1 if all mirrors are ready,
+ * 0 if some mirrors are still performing initial sync,
+ *-1 on error.
+ */
+static int
+qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+{
+size_t i;
+int ret = 1;
+
+for (i = 0; i vm-def-ndisks; i++) {
+virDomainDiskDefPtr disk = vm-def-disks[i];
+
+/* skip shared, RO and source-less disks */
+if (disk-src-shared || disk-src-readonly ||
+!virDomainDiskGetSource(disk))
+continue;
+
+/* skip disks that didn't start mirroring */
+if (!disk-blockJobSync)
+continue;
+
+/* process any pending event */
+if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk,
+0ull, NULL) 0)
+return -1;
+
+switch (disk-mirrorState) {
+case VIR_DOMAIN_DISK_MIRROR_STATE_NONE:
+ret = 0;
+break;
+case VIR_DOMAIN_DISK_MIRROR_STATE_ABORT:
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(migration of disk %s failed),
+ disk-dst);
+return -1;
+}
+}
+
+return ret;
+}
+
+
+/**
+ * qemuMigrationCancelOneDriveMirror:
+ * @driver: qemu driver
+ * @vm: domain
+ *
+ * Cancel all drive-mirrors started by qemuMigrationDriveMirror.
+ * Any pending block job events for the mirrored disks will be
+ * processed.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+static int
+qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+char *diskAlias = NULL;
+int ret = -1;
+
+/* No need to cancel if mirror already aborted */
+if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) {
+ret = 0;
+} else {
+virConnectDomainEventBlockJobStatus status = -1;
+
+if (virAsprintf(diskAlias, %s%s,
+QEMU_DRIVE_HOST_PREFIX, disk-info.alias) 0)
+goto cleanup;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm,
+ QEMU_ASYNC_JOB_MIGRATION_OUT) 0)
+goto endjob;
+ret = qemuMonitorBlockJobCancel(priv-mon, diskAlias, true);
+if (qemuDomainObjExitMonitor(driver, vm) 0)
+goto endjob;
+
+if (ret 0) {
+virDomainBlockJobInfo info;
+
+/* block-job-cancel can fail if QEMU simultaneously
+ * aborted the job; probe for it again to detect this */
+if (qemuMonitorBlockJobInfo(priv-mon, diskAlias,
+info, NULL) == 0) {
+ret = 0;
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(could not
[libvirt] Building libvirt under Windows
Hello! I am currently building libvirt under Windows64, and i would like to ask for correct way to do it. The first thing to resolve is absence of RPC XDR library. From configure source i figured out that portablexdr library may help. I have downloaded it, but it comes with a different RPC code generator named portable-rpcgen. I managed to use it, but i had to patch both libvirt (in order to recognize this generator and handle it properly) and portable-rpcgen (in order to correctly work under Windows, as well as get rid of some crashes). Initially i wanted to upstream portablexdr fixes and emailed the author. But the author replied that he is not going to fix portablexdr because it's now deprecated, since original XDR code comes under free license now. So, i have all necessary fixes in my own fork. He suggested me to come here to ask about these things. So, guys, how do you do it ? I cannot find any rpcgen package usable under Windows (MinGW, not Cygwin). Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] qemuBlockJobEventProcess: move to new source file
We will want to use synchronous block jobs from qemu_migration as well,
so split this function out into a new source file.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/Makefile.am | 1 +
src/qemu/qemu_blockjob.c | 167 +++
src/qemu/qemu_blockjob.h | 33 ++
src/qemu/qemu_driver.c | 118 +
4 files changed, 202 insertions(+), 117 deletions(-)
create mode 100644 src/qemu/qemu_blockjob.c
create mode 100644 src/qemu/qemu_blockjob.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 3c9eac6..196b0f5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -714,6 +714,7 @@ VBOX_DRIVER_EXTRA_DIST =
\
QEMU_DRIVER_SOURCES = \
qemu/qemu_agent.c qemu/qemu_agent.h \
+ qemu/qemu_blockjob.c qemu/qemu_blockjob.h \
qemu/qemu_capabilities.c qemu/qemu_capabilities.h \
qemu/qemu_command.c qemu/qemu_command.h \
qemu/qemu_domain.c qemu/qemu_domain.h \
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
new file mode 100644
index 000..e61ad8c
--- /dev/null
+++ b/src/qemu/qemu_blockjob.c
@@ -0,0 +1,167 @@
+/*
+ * qemu_blockjob.c: helper functions for QEMU block jobs
+ *
+ * Copyright (C) 2006-2015 Red Hat, Inc.
+ * Copyright (C) 2006 Daniel P. Berrange
+ *
+ * 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/.
+ */
+
+#include config.h
+
+#include internal.h
+
+#include qemu_blockjob.h
+#include qemu_domain.h
+
+#include conf/domain_conf.h
+#include conf/domain_event.h
+
+#include virlog.h
+#include virstoragefile.h
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT(qemu.qemu_blockjob);
+
+/**
+ * qemuBlockJobEventProcess:
+ * @driver: qemu driver
+ * @vm: domain
+ * @disk: domain disk
+ * @type: block job type
+ * @status: block job status
+ *
+ * Update disk's mirror state in response to a block job event
+ * from QEMU. For mirror state's that must survive libvirt
+ * restart, also update the domain's status XML.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+void
+qemuBlockJobEventProcess(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ int type,
+ int status)
+{
+virObjectEventPtr event = NULL;
+virObjectEventPtr event2 = NULL;
+const char *path;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virDomainDiskDefPtr persistDisk = NULL;
+bool save = false;
+
+/* Have to generate two variants of the event for old vs. new
+ * client callbacks */
+if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT
+disk-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+type = disk-mirrorJob;
+path = virDomainDiskGetSource(disk);
+event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
+event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type, status);
+
+/* If we completed a block pull or commit, then update the XML
+ * to match. */
+switch ((virConnectDomainEventBlockJobStatus) status) {
+case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
+if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) {
+if (vm-newDef) {
+int indx = virDomainDiskIndexByName(vm-newDef, disk-dst,
false);
+virStorageSourcePtr copy = NULL;
+
+if (indx = 0) {
+persistDisk = vm-newDef-disks[indx];
+copy = virStorageSourceCopy(disk-mirror, false);
+if (virStorageSourceInitChainElement(copy,
+ persistDisk-src,
+ true) 0) {
+VIR_WARN(Unable to update persistent definition
+ on vm %s after block job,
+ vm-def-name);
+virStorageSourceFree(copy);
+copy = NULL;
+persistDisk = NULL;
+}
+}
+if (copy) {
+
[libvirt] building xen and libvirt with odd --prefix fails
My xen is configured with --prefix=/odd/path --enable-rpath. My libvirt is configured with env PKG_CONFIG_PATH=/odd/path/share/pkgconfig bash -x autogen.sh --without-xen --with-libxl. Now make in libvirt fails to find xen/xen.h needed by xenconfig/xen_common.c, I think it expects it in /usr/include. But I have no xen-devel.rpm installed. I wonder if there are wrong assumptions in libvirt about xen. Since a while xen has a xenlight.pc which contains the required info to find headers and libs. Can there ever be a case where libxl is somewhere else than libxenstore and whatever else is provided by xen? I think they always go into the very same place. So shouldnt libvirt be updated to first check for xenlight.pc, and use the result for everything related to xen? I think that would work for every xen version which has a xenlight.pc (4.5+). And initially I did not pass the matching --prefix to libvirts configure. Even with matching --prefix libvirt does not look in $prefix/include for xen/xen.h, even with --includedir=$prefix/include. Olaf -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add source check before attaching device
On Thu, Apr 16, 2015 at 16:51:52 +0800, zhang bo wrote: Source device/file is not unique now, we should check it when attach device. The check is vary fragile and unreliable. It would be very hard (perhaps even impossible) to really detect that the two disks/filesystems point to the same source, which means libvirt would not really prevent anyone from attaching a single source twice. Moreover, the code as written would incorrectly refuse to attach disks that do not point to the same source (network disks, e.g.). But anyway, I don't think libvirt should be doing this in general, so NACK to this idea. Moreover, you should be able to achieve the same goal (and even more) by using a locking driver. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] util: client: shall libvirtd record task histories?
On Thu, Apr 16, 2015 at 02:28:52PM +0800, zhang bo wrote: On 2015/4/10 15:54, Jiri Denemark wrote: On Wed, Apr 08, 2015 at 15:40:36 +0800, zhang bo wrote: We recently encountered a problem: 1) migrate a domain 2) the client unexpectedly got *crashed* (let's take it as virsh command) 3) *libvirtd still kept migrating the domain* 4) after it's restarted, the client didn't know the guest is still migrating. The problem is that libvirtd and the client has different view of the task state. After migration, the client may wrongly think that something's wrong that the domain got unexpectedly migrated. In my opinion, libvirtd should just *execute* tasks, like the hands of a human, while clients should be the brain to *schedule and remember* tasks. So, In order to avoid this problem,we should let the client record all the taskes somewhere, and reload the states after its restart. the client may cancel or continue the task as it wishes. Libvirtd should not record the task status. Not really. It's libvirtd, the daemon, which has to remember everything. It manages the state of all domains running on a host and synchronizes all clients that want to change state of the domains. Remember, even if a client is not restarted, domains my unexpectedly migrate somewhere else because another client might have asked for it. That said, if you're implementing a higher management layer which manages domains using libvirt and you know it is going to be the only client talking directly to libvirt, you can remember the state there if you want. However, it's not something libvirt itself should or could do. But you will most likely need to synchronize the state with libvirtd in case the client is restarted. Even libvirtd has to synchronize its internal state with all running QEMU processes when it is restarted because the state might have changed. Jirka . Thank you Jirka. Let's go a step further, suppose that the client doesn't crash at step 2), it's just disconnected to libvirtd at src side. 1) client(nova) calls virDomainMigrateToURI2() to migrate a guest 2) libvirtd at src side connects to libvirtd at dest side. 3) Unfortunately, somehow, client(nova) gets disconnected to libvirtd while migrating the guest. 4) the API virDomainMigrateToURI2() returns with error in client(nova) 5) but libvirtd doesn't aware that the connection to client is broken, and keeps migrating the guest to dest. libvirtd is aware of that, but that doesn't mean it should stop the migration, if the task virDomainMigrateToURI2() got through the wire, it started migrating. 6) the guest is migrated to the dest side eventually. 7) Because the nova at src side thinks migration is not successed as step 4), the nova at the dest will consider the migrated-in guest as an unexpected running guest, and will shut it down. nova knows the exact error that occurred and thus it can differentiate between error: Cannot migrate because 'asdf' and error: XML-RPC: connection broken or whatever. If the connection was broken nova must get all new info (refresh its knowledge state) from libvirt upon new connection. The guest disappears at last, due to the previous disconnection of libvirtd client and server. Even though libvirtd remembers everything, the client at dest side still wrongly killed the guest after migration. So, how to solve this problem? Shall libvirtd keep watching its clients' connection, and cancel running jobs concerning the disconnected client immediately after the client disconnects? -- 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: bulk stats: Ignore errors from missing/inaccessible disks
On Wed, Apr 15, 2015 at 10:25:35 -0600, Eric Blake wrote: On 04/15/2015 10:19 AM, Peter Krempa wrote: Rather than erroring out make the best attempt to retrieve other data if disks are inaccessible or missing. The failure will still be logged though. Since the bulk stats API is called on multiple domains an error like this makes the API unusable. This regression was introduced by commit 596a13713420e01b20ce3dc3fdbe06d073682675 D'oh - that points to me. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1209394 --- src/qemu/qemu_driver.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) ACK. Pushed; Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Use synchronous block job events in qemu_migration
This patch series converts qemu_migration to use the synchronous block job event code introduced in commit 630ee5ac. It fixes two problems: - Drive mirroring has been broken since that commit, since the event indicating mirror readiness isn't processed while the VM object is locked. - Migration did not wait until the drive mirrors were properly cancelled, and this could cause disk corruption. Patch 1 moves qemuBlockJobEventProcess into a separate source file so it can be used by both qemu_driver and qemu_migration. Patch 2 introduces new qemuBlockJobSync* help functions to manage a synchronous block job. Patch 3 ensures that a thread waiting on a synchronous block job event is woken up should the domain crash. Patches 4 and 5 use the new synchronous block job helpers in qemu_driver and qemu_migration respectively. Michael Chapman (5): qemuBlockJobEventProcess: move to new source file qemuBlockJobSync*: introduce sync block job helpers qemuProcessStop: wake up pending sync block jobs qemuDomainBlockJobAbort: use sync block job helpers qemu: migration: use sync block job helpers po/POTFILES.in| 1 + src/Makefile.am | 1 + src/qemu/qemu_blockjob.c | 331 ++ src/qemu/qemu_blockjob.h | 49 ++ src/qemu/qemu_driver.c| 174 +++--- src/qemu/qemu_migration.c | 439 -- src/qemu/qemu_process.c | 7 + 7 files changed, 676 insertions(+), 326 deletions(-) create mode 100644 src/qemu/qemu_blockjob.c create mode 100644 src/qemu/qemu_blockjob.h -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virCondWaitUntil: calculate timespec correctly
ts.tv_nsec was off by a factor of 1000, making timeouts less than a
second in the future often expiring immediately.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/util/virthread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virthread.c b/src/util/virthread.c
index c2a9e7f..6c49515 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -164,7 +164,7 @@ int virCondWaitUntil(virCondPtr c, virMutexPtr m, unsigned
long long whenms)
struct timespec ts;
ts.tv_sec = whenms / 1000;
-ts.tv_nsec = (whenms % 1000) * 1000;
+ts.tv_nsec = (whenms % 1000) * 100;
if ((ret = pthread_cond_timedwait(c-cond, m-lock, ts)) != 0) {
errno = ret;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add source check before attaching device
Source device/file is not unique now, we should check it when attach device.
Signed-off-by: YueWenyuan yueweny...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/conf/domain_conf.c | 15 +++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 12 +++-
src/qemu/qemu_hotplug.c | 42 +++---
5 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 58b98c6..3fd729c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18191,6 +18191,21 @@ virDomainControllerDefFormat(virBufferPtr buf,
int
+virDomainFSIndexBySrc(virDomainDefPtr def, const char *src)
+{
+virDomainFSDefPtr fs;
+size_t i;
+
+for (i = 0; i def-nfss; i++) {
+fs = def-fss[i];
+if (STREQ(fs-src, src))
+return i;
+}
+return -1;
+}
+
+
+int
virDomainFSIndexByName(virDomainDefPtr def, const char *name)
{
virDomainFSDefPtr fs;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e6fa3c9..e23f289 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2804,6 +2804,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
virDomainFSDefPtr virDomainGetFilesystemForTarget(virDomainDefPtr def,
const char *target);
int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs);
+int virDomainFSIndexBySrc(virDomainDefPtr def, const char *src);
int virDomainFSIndexByName(virDomainDefPtr def, const char *name);
virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7166283..611c0d4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -274,6 +274,7 @@ virDomainDiskSourceIsBlockType;
virDomainEmulatorPinAdd;
virDomainEmulatorPinDel;
virDomainFSDefFree;
+virDomainFSIndexBySrc;
virDomainFSIndexByName;
virDomainFSInsert;
virDomainFSRemove;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cbb6e1b..3b187f0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7953,6 +7953,11 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
_(target %s already exists), disk-dst);
return -1;
}
+if (virDomainDiskIndexByName(vmdef, disk-src-path, true) = 0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(source %s already exists), disk-src-path);
+return -1;
+}
if (qemuCheckDiskConfig(disk) 0)
return -1;
if (virDomainDiskInsert(vmdef, disk))
@@ -8035,7 +8040,12 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
fs = dev-data.fs;
if (virDomainFSIndexByName(vmdef, fs-dst) = 0) {
virReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(Target already exists));
+ _(Target %s already exists), fs-dst);
+return -1;
+}
+if (virDomainFSIndexBySrc(vmdef, fs-src) = 0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(Source %s already exists), fs-src);
return -1;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2f0549e..5dd2453 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -315,12 +315,34 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver,
}
static int
+qemuDomainCheckDiskDeviceExists(virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
+{
+int ret = -1;
+size_t i;
+for (i = 0; i vm-def-ndisks; i++) {
+if (STREQ(vm-def-disks[i]-dst, disk-dst)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(target %s already exists), disk-dst);
+return ret;
+}
+if (disk-src vm-def-disks[i]-src
+disk-src-path vm-def-disks[i]-src-path
+STREQ(vm-def-disks[i]-src-path, disk-src-path)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(source %s already exists), disk-src-path);
+return ret;
+}
+}
+return 0;
+}
+
+static int
qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
-size_t i;
int ret = -1;
const char* type = virDomainDiskBusTypeToString(disk-bus);
qemuDomainObjPrivatePtr priv = vm-privateData;
@@ -338,13 +360,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
}
-for (i = 0; i vm-def-ndisks; i++) {
-if (STREQ(vm-def-disks[i]-dst, disk-dst)) {
-
[libvirt] [PATCH 4/5] qemuDomainBlockJobAbort: use sync block job helpers
The !modern code path needs to call qemuBlockJobEventProcess directly.
the modern code path will call it via qemuBlockJobSyncWait.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/qemu/qemu_driver.c | 56 ++
1 file changed, 20 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 00a4fb1..84ea817 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16269,7 +16269,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
{
virQEMUDriverPtr driver = dom-conn-privateData;
char *device = NULL;
-virDomainDiskDefPtr disk;
+virDomainDiskDefPtr disk = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
bool save = false;
int idx;
@@ -16304,14 +16304,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
goto endjob;
disk = vm-def-disks[idx];
-if (modern !async) {
-/* prepare state for event delivery. Since qemuDomainBlockPivot is
- * synchronous, but the event is delivered asynchronously we need to
- * wait too */
-disk-blockJobStatus = -1;
-disk-blockJobSync = true;
-}
-
if (disk-mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE
disk-mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -16320,11 +16312,14 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
goto endjob;
}
+if (modern !async) {
+/* prepare state for event delivery */
+qemuBlockJobSyncBegin(disk);
+}
+
if (pivot) {
-if ((ret = qemuDomainBlockPivot(driver, vm, device, disk)) 0) {
-disk-blockJobSync = false;
+if ((ret = qemuDomainBlockPivot(driver, vm, device, disk)) 0)
goto endjob;
-}
} else {
if (disk-mirror) {
disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT;
@@ -16364,36 +16359,25 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
* blockcopy and active commit, so we can hardcode the
* event to pull and let qemuBlockJobEventProcess() handle
* the rest as usual */
-disk-blockJobType = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
-disk-blockJobStatus = VIR_DOMAIN_BLOCK_JOB_CANCELED;
+qemuBlockJobEventProcess(driver, vm, disk,
+ VIR_DOMAIN_BLOCK_JOB_TYPE_PULL,
+ VIR_DOMAIN_BLOCK_JOB_CANCELED);
} else {
-while (disk-blockJobStatus == -1 disk-blockJobSync) {
-if (virCondWait(disk-blockJobSyncCond, vm-parent.lock)
0) {
-virReportSystemError(errno, %s,
- _(Unable to wait on block job sync
- condition));
-disk-blockJobSync = false;
-goto endjob;
-}
+virConnectDomainEventBlockJobStatus status = -1;
+if (qemuBlockJobSyncWait(driver, vm, disk, status) 0) {
+ret = -1;
+} else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(failed to terminate block job on disk '%s'),
+ disk-dst);
+ret = -1;
}
}
-
-qemuBlockJobEventProcess(driver, vm, disk,
- disk-blockJobType,
- disk-blockJobStatus);
-
-/* adjust the return code if we've got an explicit failure */
-if (disk-blockJobStatus == VIR_DOMAIN_BLOCK_JOB_FAILED) {
-virReportError(VIR_ERR_OPERATION_FAILED,
- _(failed to terminate block job on disk '%s'),
- disk-dst);
- ret = -1;
-}
-
-disk-blockJobSync = false;
}
endjob:
+if (disk disk-blockJobSync)
+qemuBlockJobSyncEnd(driver, vm, disk, NULL);
qemuDomainObjEndJob(driver, vm);
cleanup:
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Building libvirt under Windows
2015-04-16 11:52 GMT+02:00 Pavel Fedin p.fe...@samsung.com: Hello! I am currently building libvirt under Windows64, and i would like to ask for correct way to do it. The first thing to resolve is absence of RPC XDR library. From configure source i figured out that portablexdr library may help. I have downloaded it, but it comes with a different RPC code generator named portable-rpcgen. I managed to use it, but i had to patch both libvirt (in order to recognize this generator and handle it properly) and portable-rpcgen (in order to correctly work under Windows, as well as get rid of some crashes). Initially i wanted to upstream portablexdr fixes and emailed the author. But the author replied that he is not going to fix portablexdr because it's now deprecated, since original XDR code comes under free license now. So, i have all necessary fixes in my own fork. He suggested me to come here to ask about these things. So, guys, how do you do it ? I cannot find any rpcgen package usable under Windows (MinGW, not Cygwin). Hi, In 2010 I wrote a set of scripts to build libvirt on Windows using MinGW. Sou can find them here: https://github.com/photron/msys_setup I haven't update them in the last two years, so they might not work out-of-the-box anymore. But it might still be helpful to look at them and see how I did things. Regarding XDR: I used portablexdr. It worked for me, I can't remember if I had to patch it or not. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls
On 04/14/2015 12:27 PM, John Ferlan wrote:
On 04/13/2015 08:01 AM, Erik Skultety wrote:
This patch adds checks for empty bitmaps right after the calls of
virBitmapParse. These only include spots where set API's are called and
where domain's XML is parsed.
Also, it partially reverts commit 983f5a which added a check for
invalid nodeset 0,^0 into virBitmapParse function. This change broke
the logic, as an empty bitmap should not cause an error.
https://bugzilla.redhat.com/show_bug.cgi?id=1210545
---
src/conf/domain_conf.c | 35 +++
src/conf/numa_conf.c | 23 +++
src/qemu/qemu_driver.c | 5 +++--
src/util/virbitmap.c | 3 ---
src/xenconfig/xen_sxpr.c | 7 +++
tests/virbitmaptest.c| 13 ++---
6 files changed, 70 insertions(+), 16 deletions(-)
...
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index f247275..9a84e4c 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -524,16 +524,23 @@ static int
test10(const void *opaque ATTRIBUTE_UNUSED)
{
int ret = -1;
-virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL;
+virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL;
if (virBitmapParse(0-3,5-8,11-15, 0, b1, 20) 0 ||
virBitmapParse(4,9,10,16-19, 0, b2, 20) 0 ||
-virBitmapParse(15, 0, b3, 20) 0)
+virBitmapParse(15, 0, b3, 20) 0 ||
+virBitmapParse(0,^0, 0, b4, 20) 0)
+goto cleanup;
+
+if (!virBitmapIsAllClear(b4))
goto cleanup;
if (virBitmapOverlaps(b1, b2) ||
+virBitmapOverlaps(b1, b4) ||
virBitmapOverlaps(b2, b3) ||
-!virBitmapOverlaps(b1, b3))
+virBitmapOverlaps(b2, b4) ||
+!virBitmapOverlaps(b1, b3) ||
+virBitmapOverlaps(b3, b4))
goto cleanup;
ret = 0;
My Coverity checker was unhappy today because 'b4' is never
virBitmapFree()'d
John
Oops, thanks for pushing the fix.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] daemon: Prefix sysctl configuration file with numbers
On 15.04.2015 13:12, Jiri Denemark wrote: Apparently, files in /usr/lib/sysctl.d are usually prefixed with numbers for easier ordering. Let's be consistent with this. I chose 60 for libvirtd so that it goes after 50-default.conf. https://bugzilla.redhat.com/show_bug.cgi?id=1084876 Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Actually, %config(noreplace) for a file in /usr/lib/sysctl.d looks somewhat wrong to me and I think we should fix that too. After all the files are usually placed in /usr/lib because they are not supposed to be modified by administrators. They should be able to override the settings in /etc/. Any opinion about this? daemon/Makefile.am | 4 ++-- libvirt.spec.in| 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] building xen and libvirt with odd --prefix fails
On Thu, Apr 16, Olaf Hering wrote: And initially I did not pass the matching --prefix to libvirts configure. Even with matching --prefix libvirt does not look in $prefix/include for xen/xen.h, even with --includedir=$prefix/include. Using env CFLAGS=-I/odd/path/include CPPFLAGS=-I/odd/path/include ./configure helps with that. And I think thats the right way to do it anyway. Olaf -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] refactor virStorageBackendSCSINewLun
On Tue, Apr 07, 2015 at 04:21:01PM -0400, John Ferlan wrote: This could use some comment about inverting the logic of the retval variable. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 55 ++ 1 file changed, 20 insertions(+), 35 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] daemon: Prefix sysctl configuration file with numbers
On 04/15/2015 05:12 AM, Jiri Denemark wrote: Apparently, files in /usr/lib/sysctl.d are usually prefixed with numbers for easier ordering. Let's be consistent with this. I chose 60 for libvirtd so that it goes after 50-default.conf. https://bugzilla.redhat.com/show_bug.cgi?id=1084876 Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Actually, %config(noreplace) for a file in /usr/lib/sysctl.d looks somewhat wrong to me and I think we should fix that too. After all the files are usually placed in /usr/lib because they are not supposed to be modified by administrators. They should be able to override the settings in /etc/. Any opinion about this? I think the idea of allowing overrides from /etc makes sense - but do we actually parse up to two files, with the last parse winning? If not, that's more code to add first. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] qemu: monitor: Refactor and fix monitor checking
On Thu, Apr 16, 2015 at 14:38:25 +0200, Jiri Denemark wrote:
On Wed, Apr 15, 2015 at 16:27:35 +0200, Peter Krempa wrote:
Among all the monitor APIs some where checking if mon is NULL and some
were not. Since it's possible to have mon equal to NULL in case a second
call is attempted once entered the monitor. This requires that every
single API checks for the monitor.
This patch adds a macro that helps checking the state of the monitor and
either refactors existing checking code to use the macro or adds it in
case it was missing.
---
Notes:
Version 3:
- fixed the check in qemuMonitorGetAllBlockStatsInfo by moving it
before the allocation
- added macros for all the return value combinations
- removed 'mon' from existing VIR_DEBUG macros
src/qemu/qemu_monitor.c | 1151
---
src/qemu/qemu_monitor.h | 18 +-
2 files changed, 292 insertions(+), 877 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 1f95547..6d7562d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
...
@@ -4418,16 +3839,12 @@ int
qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon,
virHashTablePtr *info)
{
-VIR_DEBUG(mon=%p info=%p, mon, info);
+VIR_DEBUG(info=%p, info);
int ret;
*info = NULL;
-if (!mon) {
-virReportError(VIR_ERR_INVALID_ARG, %s,
- _(monitor must not be NULL));
-return -1;
-}
+QEMU_CHECK_MONITOR_JSON(mon);
s/QEMU_CHECK_MONITOR_JSON/QEMU_CHECK_MONITOR/ because of:
oh, right. I screwed up the last one :)
if (!mon-json)
return -2;
The rest looks correct.
ACK once you fix this last issue.
Pushed; Thanks.
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 v3 1/4] storage: Split out the valid path check
On Tue, Apr 07, 2015 at 04:21:00PM -0400, John Ferlan wrote:
For virStorageBackendStablePath, in order to make decisions in other code
split out the checks regarding whether the pool's target is empty, using /dev,
using /dev/, or doesn't start with /dev
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend.c | 42 +-
src/storage/storage_backend.h | 1 +
2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 9322571..77c3be3 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1675,6 +1675,25 @@
virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
return 0;
}
+/*
+ * Check how the storage pool target path is set.
+ *
Answering 'how?' with a bool seems strange to me :)
+ * Returns false if:
+ * 1. No target path set
+ * 2. Target path is /dev or /dev/
+ * 3. Target path does not start with /dev
This comment just restates the function logic, not its intention.
And it's just a matter of time until someone changes the logic without
updating the comment. I think it is not necessary to comment the
function.
+ */
+bool
+virStorageBackendPoolUseDevPath(virStoragePoolObjPtr pool)
'UseDevPath' confuses me. How about 'PoolPathIsStable' (or
PoolPathCanBeStable, since we only check if it starts with dev).
Moreover, this takes the pool as an argument, but only looks at the
path. It should either look at the pool type and deal with logical pools
as well, or just take a string as an argument.
+{
+if (pool-def-target.path == NULL ||
+STREQ(pool-def-target.path, /dev) ||
+STREQ(pool-def-target.path, /dev/) ||
+!STRPREFIX(pool-def-target.path, /dev))
+return false;
This would look better split into separate conditions, like it was in
the original function.
+
+return true;
+}
/*
* Given a volume path directly in /dev/XXX, iterate over the
@@ -1704,20 +1723,17 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
int retry = 0;
int direrr;
-/* Short circuit if pool has no target, or if its /dev */
-if (pool-def-target.path == NULL ||
-STREQ(pool-def-target.path, /dev) ||
-STREQ(pool-def-target.path, /dev/))
-goto ret_strdup;
-
-/* Skip whole thing for a pool which isn't in /dev
- * so we don't mess filesystem/dir based pools
+/*
+ * If this is a logical pool, then just strdup the passed devpath
+ * and return. Logical pools are under /dev but already have stable
+ * paths, so they don't need to search under the pool target path.
+ *
This comment is too verbose for my taste, I think the original version:
/* Logical pools are under /dev but already have stable paths */
is enough.
+ * For the SCSI/iSCSI pools, if the pool target path isn't under
+ * /dev, then just strdup the passed devpath and return so we don't
+ * mess filesystem/dir based pools
*/
SCSI/iSCSI pools are not filesystem/dir based.
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] daemon: Prefix sysctl configuration file with numbers
On Thu, Apr 16, 2015 at 06:45:44 -0600, Eric Blake wrote: On 04/15/2015 05:12 AM, Jiri Denemark wrote: Apparently, files in /usr/lib/sysctl.d are usually prefixed with numbers for easier ordering. Let's be consistent with this. I chose 60 for libvirtd so that it goes after 50-default.conf. https://bugzilla.redhat.com/show_bug.cgi?id=1084876 Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Actually, %config(noreplace) for a file in /usr/lib/sysctl.d looks somewhat wrong to me and I think we should fix that too. After all the files are usually placed in /usr/lib because they are not supposed to be modified by administrators. They should be able to override the settings in /etc/. Any opinion about this? I think the idea of allowing overrides from /etc makes sense - but do we actually parse up to two files, with the last parse winning? If not, that's more code to add first. I haven't checked, since we is the service running sysctl on boot rather than libvirt. I was hoping someone would just know how it works. I'll try to investigate. Anyway, I pushed the patch without touching the config(noreplace) part. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] qemu: lifecycle: reboot + shutdown, unexpected vm status.
On 16.04.2015 08:02, zhang bo wrote:
Steps:
1 virsh reboot guest1 --mode=acpi
2 virsh shutdown guest1 --mode=agent
Expected result:
As the SHUTDOWN job is after REBOOT, we expected the guest to be
*shutoff*. (Do you think so?)
Exacted result:
After the 2 steps above, the guest got *rebooted*.
The reason to this problem:
1 in qemuDomainReboot(mode acpi), it sets priv-fakeReboot to 1.
2 after shutdown/reboot, qemu monitor IO trigged
qemuProcessHandleShutdown(), which finds that priv-fakeReboot is 1, and
reboot the guest.
Root Cause of the problem:
After further look into the problem, We found that the design of
acpi/agent shutdown/reboot seems a little chaotic.
---
sheet1 who sets fakeReboot
---
shutdown reboot
acpiY(0) Y(1)
agent N N
It's apparently, *acpi-mode* jobs set fakeReboot.
---
sheet2 who needs to check fakeReboot(qemuProcessHandleShutdown())
---
shutdown reboot
acpiY Y
agent *Y* *N*
Things become a little odd here. only agent-mode reboot doesn't check
fakeReboot.
We can tell from the above 2 sheets, that they're not consistent.
*Agent-mode shutdown needs to check fakeReboot(trigger by SHUTDOWN
monitorIO event), while it didn't set it before.*
The chaos is not caused by libvirtd, it's a systematic problem, including
guest os and qemu. (guest os writes ACPI, triggers qemu, qemu then triggers
libvirtd)
My Solution:
A simple solution is to make the 1st sheet consistent to the 2nd sheet,
which is:
---
sheet3 who should set fakeReboot:
---
shutdown reboot
acpiY(0) Y(1)
agent *Y(0)* N
---
we let agent-mode shutdown set fakeReboot to 0.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7eb5a7d..c751dcf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1959,6 +1959,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
goto endjob;
}
+qemuDomainSetFakeReboot(driver, vm, isReboot);
+
if (useAgent) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv-agent, agentFlag);
@@ -1970,7 +1972,6 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
*/
if (!useAgent ||
(ret 0 (acpiRequested || !flags))) {
-qemuDomainSetFakeReboot(driver, vm, isReboot);
/* Even if agent failed, we have to check if guest went away
* by itself while our locks were down. */
Yes, the analysis is correct and the patch looks right. Can you please
post it among with commit message so that I can push it?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] qemu: monitor: Refactor and fix monitor checking
On Wed, Apr 15, 2015 at 16:27:35 +0200, Peter Krempa wrote:
Among all the monitor APIs some where checking if mon is NULL and some
were not. Since it's possible to have mon equal to NULL in case a second
call is attempted once entered the monitor. This requires that every
single API checks for the monitor.
This patch adds a macro that helps checking the state of the monitor and
either refactors existing checking code to use the macro or adds it in
case it was missing.
---
Notes:
Version 3:
- fixed the check in qemuMonitorGetAllBlockStatsInfo by moving it before
the allocation
- added macros for all the return value combinations
- removed 'mon' from existing VIR_DEBUG macros
src/qemu/qemu_monitor.c | 1151
---
src/qemu/qemu_monitor.h | 18 +-
2 files changed, 292 insertions(+), 877 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 1f95547..6d7562d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
...
@@ -4418,16 +3839,12 @@ int
qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon,
virHashTablePtr *info)
{
-VIR_DEBUG(mon=%p info=%p, mon, info);
+VIR_DEBUG(info=%p, info);
int ret;
*info = NULL;
-if (!mon) {
-virReportError(VIR_ERR_INVALID_ARG, %s,
- _(monitor must not be NULL));
-return -1;
-}
+QEMU_CHECK_MONITOR_JSON(mon);
s/QEMU_CHECK_MONITOR_JSON/QEMU_CHECK_MONITOR/ because of:
if (!mon-json)
return -2;
The rest looks correct.
ACK once you fix this last issue.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Building libvirt under Windows
On 04/16/2015 03:52 AM, Pavel Fedin wrote: Hello! I am currently building libvirt under Windows64, and i would like to ask for correct way to do it. The first thing to resolve is absence of RPC XDR library. From configure source i figured out that portablexdr library may help. I have downloaded it, but it comes with a different RPC code generator named portable-rpcgen. I managed to use it, but i had to patch both libvirt (in order to recognize this generator and handle it properly) and portable-rpcgen (in order to correctly work under Windows, as well as get rid of some crashes). Initially i wanted to upstream portablexdr fixes and emailed the author. But the author replied that he is not going to fix portablexdr because it's now deprecated, since original XDR code comes under free license now. So, i have all necessary fixes in my own fork. He suggested me to come here to ask about these things. So, guys, how do you do it ? I cannot find any rpcgen package usable under Windows (MinGW, not Cygwin). I haven't done much testing of the resulting project, but the ./autobuild.sh script is able to cross-build for mingw when run on a Fedora system, if you have enough of the mingw-* packages installed on Fedora. This includes mingw64-portablexdr (I have no idea if the mingw project will be shifting to a different xdr package any time soon). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virCondWaitUntil: calculate timespec correctly
On 16.04.2015 11:27, Michael Chapman wrote:
ts.tv_nsec was off by a factor of 1000, making timeouts less than a
second in the future often expiring immediately.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/util/virthread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virthread.c b/src/util/virthread.c
index c2a9e7f..6c49515 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -164,7 +164,7 @@ int virCondWaitUntil(virCondPtr c, virMutexPtr m,
unsigned long long whenms)
struct timespec ts;
ts.tv_sec = whenms / 1000;
-ts.tv_nsec = (whenms % 1000) * 1000;
+ts.tv_nsec = (whenms % 1000) * 100;
if ((ret = pthread_cond_timedwait(c-cond, m-lock, ts)) != 0) {
errno = ret;
Nice catch. ACKed and pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virCondWaitUntil: calculate timespec correctly
On Thu, Apr 16, 2015 at 03:48:25PM +0200, Martin Kletzander wrote:
On Thu, Apr 16, 2015 at 07:27:36PM +1000, Michael Chapman wrote:
ts.tv_nsec was off by a factor of 1000, making timeouts less than a
second in the future often expiring immediately.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/util/virthread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Wow, we had a bug like that there since... The introduction of that
function in commit e40438fa. That's more than 5 years.
Good catch, ACK, I'll push this shortly.
Or not, Michal pushed it already, my mail is slowed down. :)
diff --git a/src/util/virthread.c b/src/util/virthread.c
index c2a9e7f..6c49515 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -164,7 +164,7 @@ int virCondWaitUntil(virCondPtr c, virMutexPtr m, unsigned
long long whenms)
struct timespec ts;
ts.tv_sec = whenms / 1000;
-ts.tv_nsec = (whenms % 1000) * 1000;
+ts.tv_nsec = (whenms % 1000) * 100;
if ((ret = pthread_cond_timedwait(c-cond, m-lock, ts)) != 0) {
errno = ret;
--
2.1.0
--
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
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/4] scsi: Adjust return values from processLU
On Tue, Apr 07, 2015 at 04:21:03PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1171933
Adjust the processLU error returns to be a bit more logical. Currently,
the calling code cannot determine the difference between a non disk/lun
volume and a processed/found disk/lun. It can also not differentiate
between perhaps real/fatal error and one that won't necessarily stop
the code from finding other volumes.
After this patch virStorageBackendSCSIFindLUsInternal will stop processing
as soon as a fatal message occurs rather than continuting processing
for no apparent reason. It will also only set the *found value when
at least one of the processLU's was successful.
With the failed return, if the reason for the stop was that the pool
target path did not exist, was /dev, was /dev/, or did not start with
/dev, then iSCSI pool startup and refresh will fail.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_scsi.c | 44
+++---
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index d3c6470..4367b9e 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -371,6 +371,16 @@ getBlockDevice(uint32_t host,
}
+/*
+ * Process a Logical Unit entry from the scsi host device directory
+ *
+ * Returns:
+ *
+ * 0 = Found a valid entry
Maybe 1 = found, 0 = not found, but no error?
+ * -1 = Some sort of fatal error
+ * -2 = A non-fatal error such as a non disk/lun entry or an entry
Why the quotes?
Also, non-disk/cdrom isn't an error.
If we return -2 for that case as well, I'd phrase this as:
non-fatal error or a non-disk entry.
+ *without a block device found
+ */
static int
processLU(virStoragePoolObjPtr pool,
uint32_t host,
@@ -389,7 +399,7 @@ processLU(virStoragePoolObjPtr pool,
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to determine if %u:%u:%u:%u is a
Direct-Access LUN),
host, bus, target, lun);
-goto out;
+return -1;
}
/* We don't create volumes for devices other than disk and cdrom
@@ -397,32 +407,25 @@ processLU(virStoragePoolObjPtr pool,
* isn't an error, either. */
if (!(device_type == VIR_STORAGE_DEVICE_TYPE_DISK ||
device_type == VIR_STORAGE_DEVICE_TYPE_ROM))
-{
-retval = 0;
-goto out;
-}
+return -2;
VIR_DEBUG(%u:%u:%u:%u is a Direct-Access LUN,
host, bus, target, lun);
if (getBlockDevice(host, bus, target, lun, block_device) 0) {
VIR_DEBUG(Failed to find block device for this LUN);
-goto out;
+return -2;
Shouldn't this be fatal?
}
-if (virStorageBackendSCSINewLun(pool,
-host, bus, target, lun,
-block_device) 0) {
+retval = virStorageBackendSCSINewLun(pool, host, bus, target, lun,
+ block_device);
+if (retval 0)
VIR_DEBUG(Failed to create new storage volume for %u:%u:%u:%u,
host, bus, target, lun);
-goto out;
-}
-retval = 0;
-
-VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully,
- host, bus, target, lun);
+else
+VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully,
+ host, bus, target, lun);
I prefer the original logic where the success path is unindented:
if (ret 0) {
VIR_DEBUG(failure...);
goto cleanup;
}
ret = 0;
VIR_DEBUG(success)
- out:
VIR_FREE(block_device);
return retval;
}
@@ -456,6 +459,8 @@ virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr
pool,
*found = false;
while ((retval = virDirRead(devicedir, lun_dirent, device_path)) 0) {
+int rc;
+
A 'rc' variable separated from the 'ret' value of the function could be
used for the virDirRead as well
if (sscanf(lun_dirent-d_name, devicepattern,
bus, target, lun) != 3) {
continue;
@@ -463,7 +468,12 @@
virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr pool,
VIR_DEBUG(Found possible LU '%s', lun_dirent-d_name);
-if (processLU(pool, scanhost, bus, target, lun) == 0)
+rc = processLU(pool, scanhost, bus, target, lun);
+if (rc == -1) {
+retval = -1;
+break;
and this would just jump to cleanup.
+}
+if (rc == 0)
*found = true;
The int func(bool *found) signature is a bit strange,
why not just return 1 on found?
I see 'found' is used by virStoragePoolFCRefreshThread,
but there is no error checking there.
But we'd need yet another return code with the meaning of EAGAIN for
Re: [libvirt] [PATCH v4 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store
From 937126fd4075194f9fe24c46ff2d4312b8eddeea Mon Sep 17 00:00:00 2001
From: Matthias Gatto matthias.ga...@outscale.com
Date: Tue, 17 Mar 2015 20:25:37 +0100
Subject: [PATCH 4/5] virstoragefile: Always use
virStorageSourceSetBackingStore to set backing store
Replace the parts of the code where a backing store is set manually
with virStorageSourceSetBackingStore
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/conf/domain_conf.c| 3 ++-
src/conf/storage_conf.c | 16 +---
src/qemu/qemu_driver.c| 8
src/storage/storage_backend_fs.c | 6 --
src/storage/storage_backend_gluster.c | 4 ++--
src/storage/storage_backend_logical.c | 9 ++---
src/storage/storage_driver.c | 2 +-
src/util/virstoragefile.c | 8 +---
tests/virstoragetest.c| 4 ++--
9 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0800981..b210b40 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6012,7 +6012,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virDomainDiskBackingStoreParse(ctxt, backingStore) 0)
goto cleanup;
-src-backingStore = backingStore;
+if (!virStorageSourceSetBackingStore(src, backingStore, 0))
+goto cleanup;
ret = 0;
cleanup:
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 5781374..4fc248b 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1254,6 +1254,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
char *capacity = NULL;
char *unit = NULL;
char *backingStore = NULL;
+virStorageSourcePtr backingStorePtr;
xmlNodePtr node;
xmlNodePtr *nodes = NULL;
size_t i;
@@ -1290,20 +1291,21 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
}
if ((backingStore = virXPathString(string(./backingStore/path), ctxt))) {
-if (VIR_ALLOC(ret-target.backingStore) 0)
+if (VIR_ALLOC(backingStorePtr) 0)
goto error;
-ret-target.backingStore-path = backingStore;
+virStorageSourceSetBackingStore(ret-target, backingStorePtr, 0);
+backingStorePtr-path = backingStore;
backingStore = NULL;
if (options-formatFromString) {
char *format = virXPathString(string(./backingStore/format/@type), ctxt);
if (format == NULL)
-ret-target.backingStore-format = options-defaultFormat;
+backingStorePtr-format = options-defaultFormat;
else
-ret-target.backingStore-format = (options-formatFromString)(format);
+backingStorePtr-format = (options-formatFromString)(format);
-if (ret-target.backingStore-format 0) {
+if (backingStorePtr-format 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(unknown volume format type %s), format);
VIR_FREE(format);
@@ -1312,9 +1314,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
VIR_FREE(format);
}
-if (VIR_ALLOC(ret-target.backingStore-perms) 0)
+if (VIR_ALLOC(backingStorePtr-perms) 0)
goto error;
-if (virStorageDefParsePerms(ctxt, ret-target.backingStore-perms,
+if (virStorageDefParsePerms(ctxt, backingStorePtr-perms,
./backingStore/permissions,
DEFAULT_VOL_PERM_MODE) 0)
goto error;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d658ed8..7914e8d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14261,12 +14261,12 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
need_unlink = false;
-newDiskSrc-backingStore = disk-src;
+virStorageSourceSetBackingStore(newDiskSrc, disk-src, 0);
disk-src = newDiskSrc;
newDiskSrc = NULL;
if (persistDisk) {
-persistDiskSrc-backingStore = persistDisk-src;
+virStorageSourceSetBackingStore(persistDiskSrc, persistDisk-src, 0);
persistDisk-src = persistDiskSrc;
persistDiskSrc = NULL;
}
@@ -14310,13 +14310,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
tmp = disk-src;
disk-src = virStorageSourceGetBackingStore(tmp, 0);
-tmp-backingStore = NULL;
+virStorageSourceSetBackingStore(tmp, NULL, 0);
virStorageSourceFree(tmp);
if (persistDisk) {
tmp = persistDisk-src;
persistDisk-src = virStorageSourceGetBackingStore(tmp, 0);
-tmp-backingStore = NULL;
+virStorageSourceSetBackingStore(tmp, NULL, 0);
virStorageSourceFree(tmp);
}
}
diff --git
Re: [libvirt] unclear docs about scsi hostdev
On Wed, Apr 15, Olaf Hering wrote: The scsi host example in https://libvirt.org/formatdomain.html does not make it clear what adapter name='scsi_host0'/ refers to. It implies that any host #0 will be used. Since the index numbers for each host are dynamic an unexpected device may be used by libvirt. Looking through the sources it appears that adapter name=''/ is NOT a free string. Why does libvirt expect a dumb host:channel:target:lun notation for a host device? How does that work for people in practice? Olaf -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/15] Add XML files with admin API specification
No online docs are build from it since it doesn't really fit into our
document structure and new page will need to be created for it, but this
is at least a heads-up commit for easier parsing in order to build some
documentation (or python bindings) later on.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
.gitignore | 1 +
docs/Makefile.am | 23 +++
docs/apibuild.py | 10 +-
libvirt.spec.in | 1 +
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/.gitignore b/.gitignore
index 0f8b3d6..d299063 100644
--- a/.gitignore
+++ b/.gitignore
@@ -68,6 +68,7 @@
/docs/apibuild.py.stamp
/docs/devhelp/libvirt.devhelp
/docs/hvsupport.html.in
+/docs/libvirt-admin-*.xml
/docs/libvirt-api.xml
/docs/libvirt-lxc-*.xml
/docs/libvirt-qemu-*.xml
diff --git a/docs/Makefile.am b/docs/Makefile.am
index a497256..12d1cd1 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -1,6 +1,6 @@
## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2014 Red Hat, Inc.
+## Copyright (C) 2005-2015 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
@@ -128,8 +128,16 @@ lxc_xml = \
libvirt-lxc-api.xml \
libvirt-lxc-refs.xml
+admin_xml = \
+ libvirt-admin-api.xml \
+ libvirt-admin-refs.xml
+
apidir = $(pkgdatadir)/api
-api_DATA = libvirt-api.xml libvirt-qemu-api.xml libvirt-lxc-api.xml
+api_DATA = \
+ libvirt-api.xml \
+ libvirt-qemu-api.xml \
+ libvirt-lxc-api.xml \
+ libvirt-admin-api.xml
fig = \
libvirt-net-logical.fig \
@@ -149,7 +157,7 @@ EXTRA_DIST= \
hacking1.xsl hacking2.xsl wrapstring.xsl \
$(dot_html) $(dot_html_in) $(gif) $(apihtml) $(apipng) \
$(devhelphtml) $(devhelppng) $(devhelpcss) $(devhelpxsl) \
- $(xml) $(qemu_xml) $(lxc_xml) $(fig) $(png) $(css) \
+ $(xml) $(qemu_xml) $(lxc_xml) $(admin_xml) $(fig) $(png) $(css) \
$(patches) $(dot_php_in) $(dot_php_code_in) $(dot_php)\
$(internals_html_in) $(internals_html) \
sitemap.html.in aclperms.htmlinc \
@@ -174,6 +182,7 @@ all-am: web
api: $(srcdir)/libvirt-api.xml $(srcdir)/libvirt-refs.xml
qemu_api: $(srcdir)/libvirt-qemu-api.xml $(srcdir)/libvirt-qemu-refs.xml
lxc_api: $(srcdir)/libvirt-lxc-api.xml $(srcdir)/libvirt-lxc-refs.xml
+admin_api: $(srcdir)/libvirt-admin-api.xml $(srcdir)/libvirt-admin-refs.xml
web: $(dot_html) $(internals_html) html/index.html devhelp/index.html \
$(dot_php)
@@ -274,6 +283,7 @@ $(addprefix $(srcdir)/,$(devhelphtml)):
$(srcdir)/libvirt-api.xml $(devhelpxsl)
python_generated_files = \
$(srcdir)/html/libvirt-libvirt-lxc.html \
$(srcdir)/html/libvirt-libvirt-qemu.html \
+ $(srcdir)/html/libvirt-libvirt-admin.html \
$(srcdir)/html/libvirt-virterror.html \
$(srcdir)/libvirt-api.xml \
$(srcdir)/libvirt-refs.xml \
@@ -281,6 +291,8 @@ python_generated_files = \
$(srcdir)/libvirt-lxc-refs.xml \
$(srcdir)/libvirt-qemu-api.xml \
$(srcdir)/libvirt-qemu-refs.xml \
+ $(srcdir)/libvirt-admin-api.xml \
+ $(srcdir)/libvirt-admin-refs.xml \
$(NULL)
APIBUILD=$(srcdir)/apibuild.py
@@ -304,10 +316,12 @@ $(APIBUILD_STAMP): $(srcdir)/apibuild.py \
$(srcdir)/../include/libvirt/libvirt-stream.h \
$(srcdir)/../include/libvirt/libvirt-lxc.h \
$(srcdir)/../include/libvirt/libvirt-qemu.h \
+ $(srcdir)/../include/libvirt/libvirt-admin.h \
$(srcdir)/../include/libvirt/virterror.h \
$(srcdir)/../src/libvirt.c \
$(srcdir)/../src/libvirt-lxc.c \
$(srcdir)/../src/libvirt-qemu.c \
+ $(srcdir)/../src/libvirt-admin.c \
$(srcdir)/../src/util/virerror.c \
$(srcdir)/../src/util/virevent.c \
$(srcdir)/../src/util/virtypedparam.c
@@ -326,9 +340,10 @@ maintainer-clean-local: clean-local
todo.html.in
rm -rf $(srcdir)/libvirt-qemu-api.xml $(srcdir)/libvirt-qemu-refs.xml
rm -rf $(srcdir)/libvirt-lxc-api.xml $(srcdir)/libvirt-lxc-refs.xml
+ rm -rf $(srcdir)/libvirt-admin-api.xml $(srcdir)/libvirt-admin-refs.xml
rm -rf $(APIBUILD_STAMP)
-rebuild: api qemu_api lxc_api all
+rebuild: api qemu_api lxc_api admin_api all
install-data-local:
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
diff --git a/docs/apibuild.py b/docs/apibuild.py
index 9fa9361..95e9f27 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -59,6 +59,11 @@ lxc_included_files = {
libvirt-lxc.c: Implementations for the LXC specific APIs,
}
+admin_included_files = {
+ libvirt-admin.h: header with admin specific API definitions,
+ libvirt-admin.c: Implementations for the admin specific APIs,
+}
+
ignored_words = {
[libvirt] [PATCH 06/15] Add admin protocol
For now there's only CONNECT_OPEN procedure.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
.gitignore | 1 +
cfg.mk | 2 +-
src/Makefile.am| 13 +-
src/admin/admin_protocol.x | 60 ++
src/admin_protocol-structs | 8 +++
5 files changed, 82 insertions(+), 2 deletions(-)
create mode 100644 src/admin/admin_protocol.x
create mode 100644 src/admin_protocol-structs
diff --git a/.gitignore b/.gitignore
index 9d09709..56916cf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -110,6 +110,7 @@
/src/access/viraccessapichecklxc.h
/src/access/viraccessapicheckqemu.c
/src/access/viraccessapicheckqemu.h
+/src/admin/admin_protocol.[ch]
/src/esx/*.generated.*
/src/hyperv/*.generated.*
/src/libvirt*.def
diff --git a/cfg.mk b/cfg.mk
index 9ba2134..09803e4 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1,5 +1,5 @@
# Customize Makefile.maint. -*- makefile -*-
-# Copyright (C) 2008-2014 Red Hat, Inc.
+# Copyright (C) 2008-2015 Red Hat, Inc.
# Copyright (C) 2003-2008 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
diff --git a/src/Makefile.am b/src/Makefile.am
index d6245bd..b1044a7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -393,6 +393,16 @@ REMOTE_DRIVER_SOURCES =
\
EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \
$(REMOTE_DRIVER_GENERATED)
+ADMIN_PROTOCOL = $(srcdir)/admin/admin_protocol.x
+
+ADMIN_PROTOCOL_GENERATED = \
+ admin/admin_protocol.c \
+ admin/admin_protocol.h
+
+EXTRA_DIST += $(ADMIN_PROTOCOL) $(ADMIN_PROTOCOL_GENERATED)
+BUILT_SOURCES += $(ADMIN_PROTOCOL_GENERATED)
+MAINTAINERCLEANFILES += $(ADMIN_PROTOCOL_GENERATED)
+
# Ensure that we don't change the struct or member names or member ordering
# in remote_protocol.x The embedded perl below needs a few comments, and
# presumes you know what pdwtags output looks like:
@@ -2070,7 +2080,8 @@ RPC_PROBE_FILES = $(srcdir)/rpc/virnetprotocol.x \
$(srcdir)/remote/lxc_protocol.x \
$(srcdir)/remote/qemu_protocol.x \
$(srcdir)/lxc/lxc_monitor_protocol.x \
- $(srcdir)/locking/lock_protocol.x
+ $(srcdir)/locking/lock_protocol.x \
+ $(srcdir)/admin/admin_protocol.x
libvirt_functions.stp: $(RPC_PROBE_FILES) $(srcdir)/rpc/gensystemtap.pl
$(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gensystemtap.pl $(RPC_PROBE_FILES)
$@
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
new file mode 100644
index 000..3e1b28e
--- /dev/null
+++ b/src/admin/admin_protocol.x
@@ -0,0 +1,60 @@
+/* -*- c -*-
+ * admin_protocol.x: private protocol for communicating between
+ * remote_internal driver and libvirtd. This protocol is
+ * internal and may change at any time.
+ *
+ * Copyright (C) 2014-2015 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
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+%#include remote_protocol.h
+
+
+/*- Protocol. -*/
+struct admin_connect_open_args {
+unsigned int flags;
+};
+
+
+/* Define the program number, protocol version and procedure numbers here. */
+const ADMIN_PROGRAM = 0x06900690;
+const ADMIN_PROTOCOL_VERSION = 1;
+
+enum admin_procedure {
+/* Each function must be preceded by a comment providing one or
+ * more annotations:
+ *
+ * - @generate: none|client|server|both
+ *
+ * Whether to generate the dispatch stubs for the server
+ * and/or client code.
+ *
+ * - @readstream: paramnumber
+ * - @writestream: paramnumber
+ *
+ * The @readstream or @writestream annotations let daemon and src/remote
+ * create a stream. The direction is defined from the src/remote point
+ * of view. A readstream transfers data from daemon to src/remote. The
+ * paramnumber specifies at which offset the stream parameter is
inserted
+ * in the function parameter list.
+ */
+/**
+ * @generate: none
+ */
+ADMIN_PROC_CONNECT_OPEN = 1
+};
diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
new file mode 100644
index 000..6b2460d
--- /dev/null
+++ b/src/admin_protocol-structs
@@ -0,0
[libvirt] [EXAMPLE PATCH 14/15] admin: Add virAdmHello function
Just one of the simplest functions that returns string Clients: X
where X is the number of connected clients to daemon's first
subserver (the original one), so it can be tested using virsh, ipython,
etc.
The subserver is gathered by incrementing its reference
counter (similarly to getting qemu capabilities), so there is no
deadlock with admin subserver in this API.
Here you can see how functions should be named in the client (virAdm*)
and server (adm*).
There is also a parameter @flags that must be 0, which helps testing
proper error propagation into the client.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
daemon/admin_server.c | 22 ++
include/libvirt/libvirt-admin.h | 1 +
po/POTFILES.in | 1 +
src/admin/admin_protocol.x | 15 ++-
src/admin_protocol-structs | 9 +
src/libvirt-admin.c | 26 ++
src/libvirt_admin.syms | 1 +
7 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/daemon/admin_server.c b/daemon/admin_server.c
index 3f0939a..bbbef4d 100644
--- a/daemon/admin_server.c
+++ b/daemon/admin_server.c
@@ -94,4 +94,26 @@ adminDispatchConnectOpen(virNetSubServerPtr subserver
ATTRIBUTE_UNUSED,
return ret;
}
+static char *
+admHello(virNetServerPtr srv,
+ unsigned int flags)
+{
+char *ret = NULL;
+virNetSubServerPtr subsrv = NULL;
+size_t nclients;
+
+virCheckFlags(0, NULL);
+
+if (!(subsrv = virNetServerGetSubServer(srv, 0)))
+return NULL;
+
+nclients = virNetSubServerGetNClients(subsrv);
+if (virAsprintf(ret, Clients: %zu, nclients) 0)
+goto cleanup;
+
+ cleanup:
+virObjectUnref(subsrv);
+return ret;
+}
+
#include admin_dispatch.h
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 7fe03cf..f1f59b5 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -54,6 +54,7 @@ virAdmConnectPtr virAdmConnectOpen(unsigned int flags);
int virAdmConnectClose(virAdmConnectPtr conn);
int virAdmConnectRef(virAdmConnectPtr conn);
+char *virAdmHello(virAdmConnectPtr conn, unsigned int flags);
# ifdef __cplusplus
}
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0378166..ae8ba19 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -1,3 +1,4 @@
+daemon/admin_dispatch.h
daemon/admin_server.c
daemon/libvirtd-config.c
daemon/libvirtd.c
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 3e1b28e..f1addaf 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -30,6 +30,14 @@ struct admin_connect_open_args {
unsigned int flags;
};
+struct admin_hello_args {
+unsigned int flags;
+};
+
+struct admin_hello_ret {
+remote_string greeting;
+};
+
/* Define the program number, protocol version and procedure numbers here. */
const ADMIN_PROGRAM = 0x06900690;
@@ -56,5 +64,10 @@ enum admin_procedure {
/**
* @generate: none
*/
-ADMIN_PROC_CONNECT_OPEN = 1
+ADMIN_PROC_CONNECT_OPEN = 1,
+
+/**
+ * @generate: both
+ */
+ADMIN_PROC_HELLO = 2
};
diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
index 6b2460d..ed1dd04 100644
--- a/src/admin_protocol-structs
+++ b/src/admin_protocol-structs
@@ -3,6 +3,15 @@ struct admin_connect_open_args {
unsigned int flags;
};
+struct admin_hello_args {
+unsigned int flags;
+};
+
+struct admin_hello_ret {
+remote_string greeting;
+};
+
enum admin_procedure {
ADMIN_PROC_CONNECT_OPEN = 1,
+ADMIN_PROC_HELLO = 2,
};
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index e47089d..c69753c 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -335,3 +335,29 @@ virAdmConnectRef(virAdmConnectPtr conn)
return 0;
}
+
+
+/**
+ * virAdmHello:
+ * @conn: a connection object
+ * @flags: unused, 0 for now
+ *
+ * Testing function.
+ *
+ * Returns the number of connected clients as a string. Yes, as a
+ * string. Because it's just for testing.
+ */
+char *
+virAdmHello(virAdmConnectPtr conn,
+unsigned int flags)
+{
+char *ret = NULL;
+
+virResetLastError();
+
+ret = remoteAdminHello(conn, flags);
+if (!ret)
+virDispatchError(conn);
+
+return ret;
+}
diff --git a/src/libvirt_admin.syms b/src/libvirt_admin.syms
index 292433f..d6d2bc3 100644
--- a/src/libvirt_admin.syms
+++ b/src/libvirt_admin.syms
@@ -15,4 +15,5 @@ LIBVIRT_ADMIN_1.2.3 {
virAdmInitialize;
virAdmConnectOpen;
virAdmConnectClose;
+virAdmHello;
};
--
2.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [EXAMPLE PATCH 15/15] Example virt-admin
You had only one job. That's what you can say about this example
binary. In future, parts of virsh that are usable for this binary
should be split into separate shell-utils and virt-admin should gain all
the cool features of virsh without too much code addition.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
.gitignore | 2 ++
Makefile.am | 4 +--
configure.ac| 7 -
libvirt.spec.in | 2 ++
tools/virt-admin/Makefile.am| 70 +
tools/virt-admin/virt-admin.c | 68 +++
tools/virt-admin/virt-admin.pod | 43 +
7 files changed, 193 insertions(+), 3 deletions(-)
create mode 100644 tools/virt-admin/Makefile.am
create mode 100644 tools/virt-admin/virt-admin.c
create mode 100644 tools/virt-admin/virt-admin.pod
diff --git a/.gitignore b/.gitignore
index d299063..d6964f9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -173,6 +173,8 @@
/tools/virsh-*-edit.c
/tools/virt-*-validate
/tools/virt-sanlock-cleanup
+/tools/virt-admin/virt-admin.1
+/tools/virt-admin/virt-admin
/tools/wireshark/src/plugin.c
/tools/wireshark/src/libvirt
/update.log
diff --git a/Makefile.am b/Makefile.am
index 4aafe94..cdae769 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,6 +1,6 @@
## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2013 Red Hat, Inc.
+## Copyright (C) 2005-2015 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
@@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs
gnulib/tests \
examples/dominfo examples/domsuspend examples/apparmor \
examples/xml/nwfilter examples/openauth examples/systemtap \
tools/wireshark examples/dommigrate \
- examples/lxcconvert examples/domtop
+ examples/lxcconvert examples/domtop tools/virt-admin
ACLOCAL_AMFLAGS = -I m4
diff --git a/configure.ac b/configure.ac
index d13f825..aa0cc4b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1694,6 +1694,10 @@ dnl virsh libraries
VIRSH_LIBS=$VIRSH_LIBS $READLINE_LIBS
AC_SUBST([VIRSH_LIBS])
+dnl virt-admin libraries
+VIRT_ADMIN_LIBS=$VIRT_ADMIN_LIBS $READLINE_LIBS
+AC_SUBST([VIRT_ADMIN_LIBS])
+
dnl check if the network driver should be compiled
AC_ARG_WITH([network],
@@ -2825,7 +2829,8 @@ AC_CONFIG_FILES([\
examples/xml/nwfilter/Makefile \
examples/lxcconvert/Makefile \
tools/wireshark/Makefile \
-tools/wireshark/src/Makefile])
+tools/wireshark/src/Makefile \
+tools/virt-admin/Makefile])
AC_OUTPUT
AC_MSG_NOTICE([])
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 7c13588..7cfd358 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -2181,6 +2181,8 @@ exit 0
%if %{with_admin}
%files admin
%defattr(-, root, root)
+%{_mandir}/man1/virt-admin.1*
+%{_bindir}/virt-admin
%{_libdir}/%{name}/libvirt_admin.so
%endif
diff --git a/tools/virt-admin/Makefile.am b/tools/virt-admin/Makefile.am
new file mode 100644
index 000..47c9646
--- /dev/null
+++ b/tools/virt-admin/Makefile.am
@@ -0,0 +1,70 @@
+## Copyright (C) 2015 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
+## http://www.gnu.org/licenses/.
+
+AM_CPPFLAGS = \
+ -I$(top_builddir)/include -I$(top_srcdir)/include \
+ -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib \
+ -I$(top_builddir)/src -I$(top_srcdir)/src \
+ -I$(top_srcdir)/src/util\
+ -I$(top_srcdir) \
+ $(GETTEXT_CPPFLAGS)
+
+AM_LDFLAGS = \
+ $(RELRO_LDFLAGS)\
+ $(NO_INDIRECT_LDFLAGS) \
+ $(NULL)
+
+POD2MAN = pod2man -c Virtualization Support -r $(PACKAGE)-$(VERSION)
+
+EXTRA_DIST = virt-admin.pod
+
+DISTCLEANFILES =
+
+bin_PROGRAMS = virt-admin
+
+dist_man1_MANS = virt-admin.1
+
+virt_admin_SOURCES = virt-admin.c
+
+virt_admin_LDFLAGS = \
+ $(AM_LDFLAGS) \
+ $(COVERAGE_LDFLAGS) \
+ $(NULL)
+
+virt_admin_LDADD = \
+ $(STATIC_BINARIES)
[libvirt] [PATCH 05/15] Teach gendispatch how to handle admin dispatching files
Since this is just a new option for gendispatch, it looks more like a
cleanup. The only differences handled by it are connect pointers,
private pointers and API naming customs.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/gendispatch.pl | 128 ++---
1 file changed, 78 insertions(+), 50 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 199b38f..683bb92 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -49,6 +49,8 @@ my $procprefix = shift or die missing procedure prefix
argument;
my $protocol = shift or die missing protocol argument;
my @autogen;
+my $connect_ptr = $structprefix eq admin ? virAdmConnectPtr :
virConnectPtr;
+my $prefix = ($structprefix eq admin) ? adm : vir;
sub fixup_name {
my $name = shift;
@@ -78,9 +80,12 @@ sub fixup_name {
# Convert name_of_call to NameOfCall.
sub name_to_ProcName {
my $name = shift;
+my $forcefix = $structprefix eq admin;
my @elems;
-if ($name =~ /_/ || (lc $name) eq open || (lc $name) eq close) {
+
+if ($forcefix || $name =~ /_/ ||
+(lc $name) eq open || (lc $name) eq close) {
@elems = split /_/, $name;
@elems = map lc, @elems;
@elems = map ucfirst, @elems;
@@ -104,6 +109,19 @@ sub name_to_TypeName {
return $typename;
}
+sub push_privconn {
+my $args = shift;
+
+if (!@$args) {
+if ($structprefix eq admin) {
+push(@$args, priv-srv);
+} else {
+push(@$args, priv-conn);
+}
+}
+}
+
+
# Read the input file (usually remote_protocol.x) and form an
# opinion about the name, args and return type of each RPC.
my ($name, $ProcName, $id, $flags, %calls, @calls, %opts);
@@ -506,16 +524,11 @@ elsif ($mode eq server) {
virObjectUnref(snapshot);\n .
virObjectUnref(dom););
} elsif ($args_member =~ m/^(?:remote_string|remote_uuid)
(\S+)\S+;/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
-
+push_privconn(\@args_list);
push(@args_list, args-$1.$1_val);
push(@args_list, args-$1.$1_len);
} elsif ($args_member =~ m/^(?:opaque|remote_nonnull_string)
(\S+)\S+;(.*)$/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
+push_privconn(\@args_list);
my $cast = ;
my $arg_name = $1;
@@ -532,9 +545,7 @@ elsif ($mode eq server) {
push(@args_list, ${cast}args-$arg_name.${arg_name}_val);
push(@args_list, args-$arg_name.${arg_name}_len);
} elsif ($args_member =~ m/^(?:unsigned )?int (\S+)\S+;/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
+push_privconn(\@args_list);
push(@args_list, args-$1.$1_val);
push(@args_list, args-$1.$1_len);
@@ -556,35 +567,25 @@ elsif ($mode eq server) {
# just make all other array types fail
die unhandled type for argument value: $args_member;
} elsif ($args_member =~ m/^remote_uuid (\S+);/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
+push_privconn(\@args_list);
push(@args_list, (unsigned char *) args-$1);
} elsif ($args_member =~ m/^remote_string (\S+);/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
+push_privconn(\@args_list);
push(@vars_list, char *$1);
push(@optionals_list, $1);
push(@args_list, $1);
} elsif ($args_member =~ m/^remote_nonnull_string (\S+);/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
+push_privconn(\@args_list);
push(@args_list, args-$1);
} elsif ($args_member =~ m/^(unsigned )?int (\S+);/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
+push_privconn(\@args_list);
push(@args_list, args-$2);
} elsif ($args_member =~ m/^(unsigned )?hyper (\S+);/) {
-if (! @args_list) {
-push(@args_list, priv-conn);
-}
+push_privconn(\@args_list);
my $arg_name = $2;
@@ -819,9 +820,7 @@ elsif ($mode eq server) {
die
[libvirt] [PATCH 02/15] util: Add virabstracts file for keeping abstract classes
The first class in this file is going to be an abstract connection class
that holds a per-connection error inside. virConnect will be the first
child class inheriting from this one.
This is a separate file because virerror.c is going to depend on it and
putting it into datatypes along with other connect classes would create
a dependency on datatypes from the util library.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/Makefile.am | 3 +-
src/libvirt_private.syms | 5 +++
src/util/virabstracts.c | 100 +++
src/util/virabstracts.h | 57 +++
4 files changed, 164 insertions(+), 1 deletion(-)
create mode 100644 src/util/virabstracts.c
create mode 100644 src/util/virabstracts.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 3c9eac6..5763659 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,6 +1,6 @@
## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2014 Red Hat, Inc.
+## Copyright (C) 2005-2015 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
@@ -89,6 +89,7 @@ augeastest_DATA =
# These files are not related to driver APIs. Simply generic
# helper APIs for various purposes
UTIL_SOURCES = \
+ util/virabstracts.c util/virabstracts.h \
util/viralloc.c util/viralloc.h \
util/virarch.h util/virarch.c \
util/viratomic.h util/viratomic.c \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0335313..ef5877b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1035,6 +1035,11 @@ virSecurityManagerStackAddNested;
virSecurityManagerVerify;
+# util/virabstracts.h
+virClassForAbstractConnect;
+virGetAbstractConnect;
+
+
# util/viralloc.h
virAlloc;
virAllocN;
diff --git a/src/util/virabstracts.c b/src/util/virabstracts.c
new file mode 100644
index 000..f4c0829
--- /dev/null
+++ b/src/util/virabstracts.c
@@ -0,0 +1,100 @@
+/*
+ * virabstracts.c: abstract virClass definitions
+ *
+ * Copyright (C) 2015 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
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+#include config.h
+
+#include virabstracts.h
+#include virerror.h
+#include virlog.h
+#include viralloc.h
+#include viruuid.h
+#include virstring.h
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT(util);
+
+virClassPtr virAbstractConnectClass;
+
+static void virAbstractConnectDispose(void *anyobj);
+
+
+static int
+virAbstractsOnceInit(void)
+{
+#define DECLARE_CLASS(basename, parent) \
+if (!(basename ## Class = virClassNew(parent,\
+ #basename, \
+ sizeof(basename), \
+ basename ## Dispose))) \
+return -1;
+
+
+DECLARE_CLASS(virAbstractConnect, virClassForObjectLockable());
+
+
+#undef DECLARE_CLASS
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virAbstracts)
+
+
+virClassPtr
+virClassForAbstractConnect(void)
+{
+if (virAbstractsInitialize() 0)
+return NULL;
+
+return virAbstractConnectClass;
+}
+
+
+/**
+ * virGetAbstractConnect:
+ *
+ * Allocates a new abstract connection object.
+ *
+ * Returns a pointer to the connection object, or NULL on error.
+ */
+void *
+virGetAbstractConnect(virClassPtr klass)
+{
+virAbstractConnectPtr ret;
+
+if (virAbstractsInitialize() 0)
+return NULL;
+
+if (!(ret = virObjectLockableNew(klass)))
+return NULL;
+
+return ret;
+}
+
+
+static void
+virAbstractConnectDispose(void *anyobj)
+{
+virAbstractConnectPtr obj = anyobj;
+
+virResetError(obj-err);
+}
diff --git a/src/util/virabstracts.h b/src/util/virabstracts.h
new file mode 100644
index 000..4d12862
--- /dev/null
+++ b/src/util/virabstracts.h
@@ -0,0 +1,57 @@
+/*
+ * virabstracts.h: abstract virClass definitions
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General
[libvirt] [PATCH 12/15] Add support for admin API in libvirt daemon
For this to pe properly separated from other protocols used by the server, there is second subserver added which allows access to the whole virNetServer to its clients. Signed-off-by: Martin Kletzander mklet...@redhat.com --- cfg.mk| 3 ++ daemon/Makefile.am| 33 +- daemon/admin_server.c | 97 +++ daemon/admin_server.h | 36 +++ daemon/libvirtd.c | 91 ++- daemon/libvirtd.h | 14 +++- po/POTFILES.in| 1 + 7 files changed, 264 insertions(+), 11 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h diff --git a/cfg.mk b/cfg.mk index b948b7a..7c1c597 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1072,6 +1072,7 @@ sc_po_check: \ $(srcdir)/daemon/remote_dispatch.h \ $(srcdir)/daemon/qemu_dispatch.h \ $(srcdir)/src/remote/remote_client_bodies.h \ + $(srcdir)/daemon/admin_dispatch.h \ $(srcdir)/src/admin/admin_client.h $(srcdir)/daemon/remote_dispatch.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C daemon remote_dispatch.h @@ -1079,6 +1080,8 @@ $(srcdir)/daemon/qemu_dispatch.h: $(srcdir)/src/remote/qemu_protocol.x $(MAKE) -C daemon qemu_dispatch.h $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C src remote/remote_client_bodies.h +$(srcdir)/daemon/admin_server.h: $(srcdir)/src/admin/admin_protocol.x + $(MAKE) -C daemon admin_dispatch.h $(srcdir)/src/admin/admin_client.h: $(srcdir)/src/admin/admin_protocol.x $(MAKE) -C src admin/admin_client.h diff --git a/daemon/Makefile.am b/daemon/Makefile.am index bceaeb2..b309ce9 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-2015 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 @@ -25,6 +25,7 @@ INCLUDES = \ -I$(top_srcdir)/src/conf \ -I$(top_srcdir)/src/rpc \ -I$(top_srcdir)/src/remote \ + -I$(top_srcdir)/src/admin \ -I$(top_srcdir)/src/access \ $(GETTEXT_CPPFLAGS) @@ -49,6 +50,7 @@ EXTRA_DIST = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ + admin_dispatch.h\ libvirtd.conf \ libvirtd.init.in\ libvirtd.upstart\ @@ -76,6 +78,9 @@ BUILT_SOURCES = REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x LXC_PROTOCOL = $(top_srcdir)/src/remote/lxc_protocol.x QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x +ADMIN_PROTOCOL = $(top_srcdir)/src/admin/admin_protocol.x + +BUILT_SOURCES += admin_dispatch.h remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) @@ -95,6 +100,12 @@ qemu_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ --mode=server qemu QEMU $(QEMU_PROTOCOL) \ $(srcdir)/qemu_dispatch.h +admin_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \ + $(ADMIN_PROTOCOL) + $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \ + --mode=server admin ADMIN $(ADMIN_PROTOCOL) \ + $(srcdir)/admin_dispatch.h + if WITH_LIBVIRTD # Build a convenience library, for reuse in tests/libvirtdconftest @@ -114,6 +125,25 @@ libvirtd_conf_la_LDFLAGS = \ $(NULL) libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) +noinst_LTLIBRARIES += libvirtd_admin.la +libvirtd_admin_la_SOURCES = \ + admin_server.c \ + ../src/admin/admin_protocol.c + +libvirtd_admin_la_CFLAGS = \ + $(AM_CFLAGS)\ + $(XDR_CFLAGS) \ + $(PIE_CFLAGS) \ + $(WARN_CFLAGS) \ + $(LIBXML_CFLAGS)\ + $(COVERAGE_CFLAGS) + +libvirtd_admin_la_LDFLAGS = \ + $(PIE_LDFLAGS) \ + $(RELRO_LDFLAGS)\ + $(COVERAGE_LDFLAGS) \ + $(NO_INDIRECT_LDFLAGS) + man8_MANS = libvirtd.8 sbin_PROGRAMS = libvirtd @@ -166,6 +196,7 @@ endif WITH_DTRACE_PROBES libvirtd_LDADD += \ libvirtd_conf.la \ + libvirtd_admin.la \ ../src/libvirt-lxc.la \ ../src/libvirt-qemu.la \ ../src/libvirt_driver_remote.la \ diff --git a/daemon/admin_server.c b/daemon/admin_server.c new file mode 100644 index 000..3f0939a --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,97 @@ +/* + *
[libvirt] [PATCH 08/15] Add admin error domain
Just the addition of VIR_FROM_ADMIN to the enum of error domains.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
include/libvirt/virterror.h | 3 ++-
src/util/virerror.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 9c5b069..6325030 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -4,7 +4,7 @@
* Description: Provides the interfaces of the libvirt library to handle
* errors raised while using the library.
*
- * Copyright (C) 2006, 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2015 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
@@ -126,6 +126,7 @@ typedef enum {
VIR_FROM_POLKIT = 60, /* Error from polkit code */
VIR_FROM_THREAD = 61, /* Error from thread utils */
+VIR_FROM_ADMIN = 62,/* Error from admin backend */
# ifdef VIR_ENUM_SENTINELS
VIR_ERR_DOMAIN_LAST
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 8696e31..18c6c2a 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -134,6 +134,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
Polkit, /* 60 */
Thread jobs,
+ Admin Interface,
)
--
2.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/15] datatypes: Use abstract connect class in virConnect
Make virConnectClass be a child of virAbstractConnectClass and adapt all
error handling functions to that. This will allow us to create new
connect classes that will just work with our error reporting.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/Makefile.am | 1 +
src/datatypes.c | 9 ++---
src/datatypes.h | 8 ++---
src/libvirt-host.c | 3 +-
src/util/virerror.c | 94 +++--
src/util/virerror.h | 4 +--
6 files changed, 81 insertions(+), 38 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 5763659..6d1b4fb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2122,6 +2122,7 @@ if WITH_LXC
noinst_LTLIBRARIES += libvirt-setuid-rpc-client.la
libvirt_setuid_rpc_client_la_SOURCES = \
+ util/virabstracts.c \
util/viralloc.c \
util/viratomic.c\
util/viratomic.h\
diff --git a/src/datatypes.c b/src/datatypes.c
index 39f83d9..b21113e 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -72,8 +72,10 @@ virDataTypesOnceInit(void)
DECLARE_CLASS_COMMON(basename, virClassForObject())
#define DECLARE_CLASS_LOCKABLE(basename) \
DECLARE_CLASS_COMMON(basename, virClassForObjectLockable())
+#define DECLARE_CLASS_CONNECT(basename) \
+DECLARE_CLASS_COMMON(basename, virClassForAbstractConnect())
-DECLARE_CLASS_LOCKABLE(virConnect);
+DECLARE_CLASS_CONNECT(virConnect);
DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData);
DECLARE_CLASS(virDomain);
DECLARE_CLASS(virDomainSnapshot);
@@ -88,6 +90,7 @@ virDataTypesOnceInit(void)
#undef DECLARE_CLASS_COMMON
#undef DECLARE_CLASS_LOCKABLE
+#undef DECLARE_CLASS_CONNECT
#undef DECLARE_CLASS
return 0;
@@ -110,7 +113,7 @@ virGetConnect(void)
if (virDataTypesInitialize() 0)
return NULL;
-if (!(ret = virObjectLockableNew(virConnectClass)))
+if (!(ret = virGetAbstractConnect(virConnectClass)))
return NULL;
if (!(ret-closeCallback =
virObjectLockableNew(virConnectCloseCallbackDataClass)))
@@ -138,8 +141,6 @@ virConnectDispose(void *obj)
if (conn-driver)
conn-driver-connectClose(conn);
-virResetError(conn-err);
-
virURIFree(conn-uri);
if (conn-closeCallback) {
diff --git a/src/datatypes.h b/src/datatypes.h
index f1d01d5..9f95811 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -25,6 +25,7 @@
# include internal.h
# include driver.h
+# include virabstracts.h
# include virthread.h
# include virobject.h
# include viruuid.h
@@ -328,7 +329,7 @@ struct _virConnectCloseCallbackData {
* Internal structure associated to a connection
*/
struct _virConnect {
-virObjectLockable object;
+virAbstractConnect parent;
/* All the variables from here, until declared otherwise in one of
* the following comments, are setup at time of connection open
@@ -359,11 +360,6 @@ struct _virConnect {
* virDomain/virNetwork object associated with this connection.
*/
-/* Per-connection error. */
-virError err; /* the last error */
-virErrorFunc handler; /* associated handlet */
-void *userData; /* the user data */
-
/* Per-connection close callback */
virConnectCloseCallbackDataPtr closeCallback;
};
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index 03bee1f..74d9bef 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -51,7 +51,8 @@ VIR_LOG_INIT(libvirt.host);
int
virConnectRef(virConnectPtr conn)
{
-VIR_DEBUG(conn=%p refs=%d, conn, conn ? conn-object.parent.u.s.refs :
0);
+VIR_DEBUG(conn=%p refs=%d, conn,
+ conn ? conn-parent.parent.parent.u.s.refs : 0);
virResetLastError();
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 73dae95..8696e31 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -27,6 +27,7 @@
#include string.h
#include stdarg.h
+#include virabstracts.h
#include virerror.h
#include datatypes.h
#include virlog.h
@@ -421,9 +422,10 @@ virResetLastError(void)
virErrorPtr
virConnGetLastError(virConnectPtr conn)
{
-if (conn == NULL)
+if (!conn)
return NULL;
-return conn-err;
+
+return conn-parent.err;
}
/**
@@ -458,17 +460,38 @@ virConnCopyLastError(virConnectPtr conn, virErrorPtr to)
/* We can't guarantee caller has initialized it to zero */
memset(to, 0, sizeof(*to));
-if (conn == NULL)
+if (!conn)
return -1;
+
virObjectLock(conn);
-if (conn-err.code == VIR_ERR_OK)
+if (conn-parent.err.code == VIR_ERR_OK)
virResetError(to);
else
-virCopyError(conn-err, to);
+virCopyError(conn-parent.err, to);
virObjectUnlock(conn);
return to-code;
}
+static void
+virAdmConnResetLastError(void *anyobj)
+{
+virAbstractConnectPtr obj = anyobj;
+
+
[libvirt] [PATCH 01/15] util: add virJSONValueCopy
Faster version of virJSONValueFromString(virJSONValueToString()).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/virjson.c | 65 ++-
src/util/virjson.h | 4 +-
tests/jsontest.c | 111 +++
4 files changed, 179 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index aafc385..0335313 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1573,6 +1573,7 @@ virJSONValueArrayAppend;
virJSONValueArrayGet;
virJSONValueArraySize;
virJSONValueArraySteal;
+virJSONValueCopy;
virJSONValueFree;
virJSONValueFromString;
virJSONValueGetArrayAsBitmap;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index c8d761f..40ec613 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1,7 +1,7 @@
/*
* virjson.c: JSON object parsing/formatting
*
- * Copyright (C) 2009-2010, 2012-2013 Red Hat, Inc.
+ * Copyright (C) 2009-2010, 2012-2015 Red Hat, Inc.
* Copyright (C) 2009 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -1233,6 +1233,69 @@ virJSONValueObjectForeachKeyValue(virJSONValuePtr object,
}
+virJSONValuePtr
+virJSONValueCopy(virJSONValuePtr in)
+{
+size_t i;
+virJSONValuePtr out = NULL;
+
+if (!in)
+return NULL;
+
+switch ((virJSONType) in-type) {
+case VIR_JSON_TYPE_OBJECT:
+out = virJSONValueNewObject();
+if (!out)
+return NULL;
+for (i = 0; i in-data.object.npairs; i++) {
+virJSONValuePtr val = NULL;
+if (!(val = virJSONValueCopy(in-data.object.pairs[i].value)))
+goto error;
+if (virJSONValueObjectAppend(out, in-data.object.pairs[i].key,
+ val) 0) {
+virJSONValueFree(val);
+goto error;
+}
+}
+break;
+case VIR_JSON_TYPE_ARRAY:
+out = virJSONValueNewArray();
+if (!out)
+return NULL;
+for (i = 0; i in-data.array.nvalues; i++) {
+virJSONValuePtr val = NULL;
+if (!(val = virJSONValueCopy(in-data.array.values[i])))
+goto error;
+if (virJSONValueArrayAppend(out, val) 0) {
+virJSONValueFree(val);
+goto error;
+}
+}
+break;
+
+/* No need to error out in the following cases */
+case VIR_JSON_TYPE_STRING:
+out = virJSONValueNewString(in-data.string);
+break;
+case VIR_JSON_TYPE_NUMBER:
+out = virJSONValueNewNumber(in-data.number);
+break;
+case VIR_JSON_TYPE_BOOLEAN:
+out = virJSONValueNewBoolean(in-data.boolean);
+break;
+case VIR_JSON_TYPE_NULL:
+out = virJSONValueNewNull();
+break;
+}
+
+return out;
+
+ error:
+virJSONValueFree(out);
+return NULL;
+}
+
+
#if WITH_YAJL
static int
virJSONParserInsertValue(virJSONParserPtr parser,
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 9bb7461..e871b2e 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -1,7 +1,7 @@
/*
* virjson.h: JSON object parsing/formatting
*
- * Copyright (C) 2009, 2012-2013 Red Hat, Inc.
+ * Copyright (C) 2009, 2012-2015 Red Hat, Inc.
* Copyright (C) 2009 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -165,4 +165,6 @@ int virJSONValueObjectForeachKeyValue(virJSONValuePtr
object,
virJSONValueObjectIteratorFunc cb,
void *opaque);
+virJSONValuePtr virJSONValueCopy(virJSONValuePtr in);
+
#endif /* __VIR_JSON_H_ */
diff --git a/tests/jsontest.c b/tests/jsontest.c
index a2a42e3..fca960f 100644
--- a/tests/jsontest.c
+++ b/tests/jsontest.c
@@ -131,6 +131,91 @@ testJSONAddRemove(const void *data)
static int
+testJSONCopy(const void *data)
+{
+const struct testInfo *info = data;
+virJSONValuePtr json = NULL;
+virJSONValuePtr jsonCopy = NULL;
+char *result = NULL;
+char *resultCopy = NULL;
+int ret = -1;
+
+json = virJSONValueFromString(info-doc);
+if (!json) {
+if (virTestGetVerbose())
+fprintf(stderr, Failed to parse %s\n, info-doc);
+ret = -1;
+goto cleanup;
+}
+
+jsonCopy = virJSONValueCopy(json);
+if (!jsonCopy) {
+if (virTestGetVerbose())
+fprintf(stderr, Failed to copy JSON data\n);
+ret = -1;
+goto cleanup;
+}
+
+result = virJSONValueToString(json, false);
+if (!result) {
+if (virTestGetVerbose())
+fprintf(stderr, Failed to format original JSON data\n);
+ret = -1;
+goto cleanup;
+}
+
+resultCopy = virJSONValueToString(json, false);
+if (!resultCopy) {
+if (virTestGetVerbose())
+
[libvirt] [RFCv2 PoCv1 PATCH 00/15] Just Admin API
*** BLURB *** ... Just kidding :) Sooo... After very very VERY long time, here's a draft of an admin interface that is supposed to open up new possibilities to be done on a live daemon. The aim here is to create some first inches of that API in order to open up the possibility of new API function creation to more people. I already spent so much time explaining so much of that that I don't know what else to point at in here. Maybe the fact that last three patches are just an example on how this might work. Of course there won't be any functions like listClientIDs with the need of getting each client info by another API. There's going to be e.g. virAdmClientInfo and virAdmGetClients() will return the list of them, etc. Long story short, let's not repeat past mistakes ;) With all that said, feel free to hate, love, comment or just try out compiling this series and let me know how many things I've missed and screwed up (hopefully zero). Martin Kletzander (15): util: add virJSONValueCopy util: Add virabstracts file for keeping abstract classes datatypes: Use abstract connect class in virConnect Break virNetServer into virNetSubServers Teach gendispatch how to handle admin dispatching files Add admin protocol Build client headers for admin protocol Add admin error domain Add libvirt-admin library Add XML files with admin API specification Add configuration options for permissions on daemon's admin socket Add support for admin API in libvirt daemon rpc: Add virNetSubServerGetNClients admin: Add virAdmHello function Example virt-admin .gitignore | 5 + Makefile.am| 4 +- cfg.mk | 11 +- configure.ac | 19 +- daemon/Makefile.am | 33 +- daemon/admin_server.c | 119 + daemon/admin_server.h | 36 ++ daemon/libvirtd-config.c | 5 +- daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug| 1 + daemon/libvirtd.c | 138 -- daemon/libvirtd.conf | 8 + daemon/libvirtd.h | 14 +- daemon/remote.c| 194 - daemon/test_libvirtd.aug.in| 1 + docs/Makefile.am | 23 +- docs/apibuild.py | 10 +- include/libvirt/Makefile.am| 6 +- include/libvirt/libvirt-admin.h| 63 +++ include/libvirt/virterror.h| 3 +- libvirt-admin.pc.in| 13 + libvirt.spec.in| 10 + po/POTFILES.in | 3 + src/Makefile.am| 134 +- src/admin/admin_protocol.x | 73 src/admin_protocol-structs | 17 + src/datatypes.c| 39 +- src/datatypes.h| 45 +- src/internal.h | 1 + src/libvirt-admin.c| 363 src/libvirt-host.c | 3 +- src/libvirt_admin.syms | 19 + src/libvirt_private.syms | 6 + src/libvirt_remote.syms| 17 +- src/locking/lock_daemon.c | 40 +- src/locking/lock_daemon_dispatch.c | 18 +- src/lxc/lxc_controller.c | 18 +- src/rpc/gendispatch.pl | 141 +++--- src/rpc/virnetserver.c | 859 + src/rpc/virnetserver.h | 61 +-- src/rpc/virnetserverprogram.c | 12 +- src/rpc/virnetserverprogram.h | 9 +- src/rpc/virnetsubserver.c | 672 + src/rpc/virnetsubserver.h | 78 src/util/virabstracts.c| 100 + src/util/virabstracts.h| 57 +++ src/util/virerror.c| 95 ++-- src/util/virerror.h| 4 +- src/util/virjson.c | 65 ++- src/util/virjson.h | 4 +- tests/confdata/libvirtd.conf | 6 + tests/confdata/libvirtd.out| 5 + tests/jsontest.c | 111 + tools/virt-admin/Makefile.am | 70 +++ tools/virt-admin/virt-admin.c | 68 +++ tools/virt-admin/virt-admin.pod| 43 ++ 56 files changed, 3092 insertions(+), 881 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h create mode 100644 include/libvirt/libvirt-admin.h create mode 100644 libvirt-admin.pc.in create mode 100644 src/admin/admin_protocol.x create mode 100644 src/admin_protocol-structs create mode 100644 src/libvirt-admin.c create mode 100644 src/libvirt_admin.syms create mode 100644 src/rpc/virnetsubserver.c create mode 100644 src/rpc/virnetsubserver.h create mode 100644 src/util/virabstracts.c create mode 100644 src/util/virabstracts.h create mode 100644 tools/virt-admin/Makefile.am create mode 100644 tools/virt-admin/virt-admin.c create mode 100644 tools/virt-admin/virt-admin.pod -- 2.3.5 -- libvir-list mailing list
[libvirt] [PATCH 09/15] Add libvirt-admin library
Initial scratch of the admin library. It has its own virAdmConnectPtr
that inherits from virAbstractConnectPtr and thus trivially supports
error reporting.
Configuration option --with-admin is added to control whether the admin
library should be built or not (set to 'yes' by default).
There's pkg-config file added and spec-file adjusted as well.
Since the library should be minimalistic and not depend on any other
library, the list of files is especially crafted for it. Most of them
could've been put to it's own sub-libraries that would be LIBADD'd to
libvirt_util, libvirt_net_rpc and libvirt_setuid_rpc_client to minimize
the number of object files being built, but that's a refactoring that
isn't the orginal aim of this commit.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
cfg.mk | 1 +
configure.ac| 12 ++
include/libvirt/Makefile.am | 6 +-
include/libvirt/libvirt-admin.h | 62
libvirt-admin.pc.in | 13 ++
libvirt.spec.in | 7 +
src/Makefile.am | 109 -
src/datatypes.c | 30
src/datatypes.h | 37 +
src/internal.h | 1 +
src/libvirt-admin.c | 337
src/libvirt_admin.syms | 18 +++
12 files changed, 631 insertions(+), 2 deletions(-)
create mode 100644 include/libvirt/libvirt-admin.h
create mode 100644 libvirt-admin.pc.in
create mode 100644 src/libvirt-admin.c
create mode 100644 src/libvirt_admin.syms
diff --git a/cfg.mk b/cfg.mk
index 8f20f9b..b948b7a 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -304,6 +304,7 @@ sc_flags_usage:
$(srcdir)/include/libvirt/virterror.h \
$(srcdir)/include/libvirt/libvirt-qemu.h\
$(srcdir)/include/libvirt/libvirt-lxc.h \
+ $(srcdir)/include/libvirt/libvirt-admin.h \
| grep -c '\(long\|unsigned\) flags') != 4 \
{ echo '$(ME): new API should use unsigned int flags' 12; \
exit 1; } || :
diff --git a/configure.ac b/configure.ac
index aed0934..d13f825 100644
--- a/configure.ac
+++ b/configure.ac
@@ -583,6 +583,10 @@ AC_ARG_WITH([pm-utils],
[AS_HELP_STRING([--with-pm-utils],
[use pm-utils for power management @:@default=yes@:@])])
m4_divert_text([DEFAULTS], [with_pm_utils=check])
+AC_ARG_WITH([admin],
+ [AS_HELP_STRING([--with-admin],
+[build admin library @:@default=yes@:@])])
+m4_divert_text([DEFAULTS], [with_admin=yes])
dnl
dnl in case someone want to build static binaries
@@ -821,6 +825,10 @@ if test $with_libvirtd = yes ; then
fi
AM_CONDITIONAL([WITH_LIBVIRTD], [test $with_libvirtd = yes])
+if test $with_admin = yes ; then
+AC_DEFINE_UNQUOTED([WITH_ADMIN], 1, [whether admin library is built])
+fi
+AM_CONDITIONAL([WITH_ADMIN], [test $with_admin = yes])
old_LIBS=$LIBS
old_CFLAGS=$CFLAGS
@@ -2358,6 +2366,7 @@ WIN32_EXTRA_CFLAGS=
dnl libvirt.syms is generated in builddir, but libvirt_qemu.syms is in git;
dnl hence the asymmetric naming of these two symbol files.
LIBVIRT_SYMBOL_FILE=libvirt.syms
+LIBVIRT_ADMIN_SYMBOL_FILE='$(srcdir)/libvirt_admin.syms'
LIBVIRT_LXC_SYMBOL_FILE='$(srcdir)/libvirt_lxc.syms'
LIBVIRT_QEMU_SYMBOL_FILE='$(srcdir)/libvirt_qemu.syms'
MSCOM_LIBS=
@@ -2388,6 +2397,7 @@ case $host in
# Also set the symbol file to .def, so src/Makefile generates libvirt.def
# from libvirt.syms and passes libvirt.def instead of libvirt.syms to the
linker
LIBVIRT_SYMBOL_FILE=libvirt.def
+LIBVIRT_ADMIN_SYMBOL_FILE=libvirt_admin.def
LIBVIRT_LXC_SYMBOL_FILE=libvirt_lxc.def
LIBVIRT_QEMU_SYMBOL_FILE=libvirt_qemu.def
# mingw's ld has the --version-script parameter, but it requires a .def
file
@@ -2403,6 +2413,7 @@ AC_SUBST([CYGWIN_EXTRA_LIBADD])
AC_SUBST([MINGW_EXTRA_LDFLAGS])
AC_SUBST([WIN32_EXTRA_CFLAGS])
AC_SUBST([LIBVIRT_SYMBOL_FILE])
+AC_SUBST([LIBVIRT_ADMIN_SYMBOL_FILE])
AC_SUBST([LIBVIRT_LXC_SYMBOL_FILE])
AC_SUBST([LIBVIRT_QEMU_SYMBOL_FILE])
AC_SUBST([VERSION_SCRIPT_FLAGS])
@@ -2980,6 +2991,7 @@ AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE])
AC_MSG_NOTICE([ Init script: $with_init_script])
AC_MSG_NOTICE([Char device locks: $with_chrdev_lock_files])
AC_MSG_NOTICE([ Default Editor: $DEFAULT_EDITOR])
+AC_MSG_NOTICE([Admin library: $with_admin])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Developer Tools])
AC_MSG_NOTICE([])
diff --git a/include/libvirt/Makefile.am b/include/libvirt/Makefile.am
index 8aee5d7..03c7ed7 100644
--- a/include/libvirt/Makefile.am
+++ b/include/libvirt/Makefile.am
@@ -1,6 +1,6 @@
## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2011, 2013 Red Hat, Inc.
+## Copyright (C) 2005-2011, 2013, 2014 Red Hat, Inc.
##
## This library is free software; you can redistribute it and/or
## modify it under the terms of the
[libvirt] [EXAMPLE PATCH 13/15] rpc: Add virNetSubServerGetNClients
This function accesses the number of connected clients while properly
locking the subserver it returns the data about.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetsubserver.c | 10 ++
src/rpc/virnetsubserver.h | 2 ++
3 files changed, 13 insertions(+)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index f7cf226..8501b90 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -209,6 +209,7 @@ virNetSocketWrite;
virNetSubServerAddProgram;
virNetSubServerAddService;
virNetSubServerClose;
+virNetSubServerGetNClients;
virNetSubServerHasClients;
virNetSubServerKeepAliveRequired;
virNetSubServerNew;
diff --git a/src/rpc/virnetsubserver.c b/src/rpc/virnetsubserver.c
index 8d9defb..a2c6568 100644
--- a/src/rpc/virnetsubserver.c
+++ b/src/rpc/virnetsubserver.c
@@ -660,3 +660,13 @@ virNetSubServerProcessClients(virNetSubServerPtr subsrv)
virObjectUnlock(subsrv);
}
+
+size_t
+virNetSubServerGetNClients(virNetSubServerPtr subsrv)
+{
+size_t ret = 0;
+virObjectLock(subsrv);
+ret = subsrv-nclients;
+virObjectUnlock(subsrv);
+return ret;
+}
diff --git a/src/rpc/virnetsubserver.h b/src/rpc/virnetsubserver.h
index cf7c213..0b943f8 100644
--- a/src/rpc/virnetsubserver.h
+++ b/src/rpc/virnetsubserver.h
@@ -73,4 +73,6 @@ void virNetSubServerProcessClients(virNetSubServerPtr subsrv);
void virNetSubServerUpdateServices(virNetSubServerPtr subsrv, bool enabled);
+size_t virNetSubServerGetNClients(virNetSubServerPtr subsrv);
+
#endif /* __VIR_NET_SUBSERVER_H__ */
--
2.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/15] Break virNetServer into virNetSubServers
Each subserver has its own RPC programs, services, workers, keepalive,
clients etc. Hence (possible) multiple subservers are properly
separated.
The part in remote.c is just mechanical, the same applies to most of the
code movement from virnetserver.c to virnetsubserver.c.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
daemon/libvirtd.c | 47 +-
daemon/remote.c| 194 -
po/POTFILES.in | 1 +
src/Makefile.am| 1 +
src/libvirt_remote.syms| 16 +-
src/locking/lock_daemon.c | 40 +-
src/locking/lock_daemon_dispatch.c | 18 +-
src/lxc/lxc_controller.c | 18 +-
src/rpc/gendispatch.pl | 13 +-
src/rpc/virnetserver.c | 859 +
src/rpc/virnetserver.h | 61 +--
src/rpc/virnetserverprogram.c | 12 +-
src/rpc/virnetserverprogram.h | 9 +-
src/rpc/virnetsubserver.c | 662
src/rpc/virnetsubserver.h | 76
15 files changed, 1263 insertions(+), 764 deletions(-)
create mode 100644 src/rpc/virnetsubserver.c
create mode 100644 src/rpc/virnetsubserver.h
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 107b88d..e209e93 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1,7 +1,7 @@
/*
* libvirtd.c: daemon start of day, guest process i/o management
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -492,14 +492,14 @@ daemonSetupNetworking(virNetServerPtr srv,
goto error;
}
-if (virNetServerAddService(srv, svc,
+if (virNetServerAddService(srv, 0, svc,
config-mdns_adv !ipsock ?
_libvirt._tcp :
NULL) 0)
goto error;
if (svcRO
-virNetServerAddService(srv, svcRO, NULL) 0)
+virNetServerAddService(srv, 0, svcRO, NULL) 0)
goto error;
if (ipsock) {
@@ -517,7 +517,7 @@ daemonSetupNetworking(virNetServerPtr srv,
config-max_client_requests)))
goto error;
-if (virNetServerAddService(srv, svcTCP,
+if (virNetServerAddService(srv, 0, svcTCP,
config-mdns_adv ? _libvirt._tcp :
NULL) 0)
goto error;
}
@@ -559,7 +559,7 @@ daemonSetupNetworking(virNetServerPtr srv,
virObjectUnref(ctxt);
goto error;
}
-if (virNetServerAddService(srv, svcTLS,
+if (virNetServerAddService(srv, 0, svcTLS,
config-mdns_adv
!config-listen_tcp ? _libvirt._tcp :
NULL) 0)
goto error;
@@ -1334,19 +1334,24 @@ int main(int argc, char **argv) {
goto cleanup;
}
-if (!(srv = virNetServerNew(config-min_workers,
-config-max_workers,
-config-prio_workers,
-config-max_clients,
-config-max_anonymous_clients,
-config-keepalive_interval,
-config-keepalive_count,
-!!config-keepalive_required,
-config-mdns_adv ? config-mdns_name : NULL,
-remoteClientInitHook,
-NULL,
-remoteClientFreeFunc,
-NULL))) {
+if (!(srv = virNetServerNew(config-mdns_adv ? config-mdns_name : NULL)))
{
+ret = VIR_DAEMON_ERR_INIT;
+goto cleanup;
+}
+
+if (virNetServerAddSubServer(srv,
+ config-min_workers,
+ config-max_workers,
+ config-prio_workers,
+ config-max_clients,
+ config-max_anonymous_clients,
+ config-keepalive_interval,
+ config-keepalive_count,
+ !!config-keepalive_required,
+ remoteClientInitHook,
+ NULL,
+ remoteClientFreeFunc,
+ NULL) 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1374,7 +1379,7 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
-if (virNetServerAddProgram(srv, remoteProgram) 0) {
+if (virNetServerAddProgram(srv, 0, remoteProgram) 0) {
ret =
[libvirt] [PATCH 11/15] Add configuration options for permissions on daemon's admin socket
This is not going to be very widely used, but for some corner cases and
easier (unsafe) debugging, it might be nice.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
daemon/libvirtd-config.c | 5 -
daemon/libvirtd-config.h | 1 +
daemon/libvirtd.aug | 1 +
daemon/libvirtd.conf | 8
daemon/test_libvirtd.aug.in | 1 +
tests/confdata/libvirtd.conf | 6 ++
tests/confdata/libvirtd.out | 5 +
7 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 3694455..bce2d70 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -264,7 +264,8 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
if (VIR_STRDUP(data-unix_sock_rw_perms,
data-auth_unix_rw == REMOTE_AUTH_POLKIT ? 0777 : 0700)
0 ||
-VIR_STRDUP(data-unix_sock_ro_perms, 0777) 0)
+VIR_STRDUP(data-unix_sock_ro_perms, 0777) 0 ||
+VIR_STRDUP(data-unix_sock_admin_perms, 0700) 0)
goto error;
#if WITH_SASL
@@ -337,6 +338,7 @@ daemonConfigFree(struct daemonConfig *data)
}
VIR_FREE(data-access_drivers);
+VIR_FREE(data-unix_sock_admin_perms);
VIR_FREE(data-unix_sock_ro_perms);
VIR_FREE(data-unix_sock_rw_perms);
VIR_FREE(data-unix_sock_group);
@@ -404,6 +406,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
goto error;
GET_CONF_STR(conf, filename, unix_sock_group);
+GET_CONF_STR(conf, filename, unix_sock_admin_perms);
GET_CONF_STR(conf, filename, unix_sock_ro_perms);
GET_CONF_STR(conf, filename, unix_sock_rw_perms);
diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h
index c996995..b8d2bc0 100644
--- a/daemon/libvirtd-config.h
+++ b/daemon/libvirtd-config.h
@@ -35,6 +35,7 @@ struct daemonConfig {
char *tls_port;
char *tcp_port;
+char *unix_sock_admin_perms;
char *unix_sock_ro_perms;
char *unix_sock_rw_perms;
char *unix_sock_group;
diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
index 5a0807c..b20ceca 100644
--- a/daemon/libvirtd.aug
+++ b/daemon/libvirtd.aug
@@ -35,6 +35,7 @@ module Libvirtd =
let sock_acl_entry = str_entry unix_sock_group
| str_entry unix_sock_ro_perms
| str_entry unix_sock_rw_perms
+ | str_entry unix_sock_admin_perms
| str_entry unix_sock_dir
let authentication_entry = str_entry auth_unix_ro
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 069ef3a..6ef38fa 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -106,9 +106,17 @@
# control, then you may want to relax this too.
#unix_sock_rw_perms = 0770
+# Set the UNIX socket permissions for the admin interface socket.
+#
+# Default allows only owner (root), do not change it unless you are
+# sure to whom you are exposing the access to.
+#unix_sock_admin_perms = 0700
+
# Set the name of the directory in which sockets will be found/created.
#unix_sock_dir = /var/run/libvirt
+
+
#
#
# Authentication.
diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in
index 37ff33d..a87df5f 100644
--- a/daemon/test_libvirtd.aug.in
+++ b/daemon/test_libvirtd.aug.in
@@ -12,6 +12,7 @@ module Test_libvirtd =
{ unix_sock_group = libvirt }
{ unix_sock_ro_perms = 0777 }
{ unix_sock_rw_perms = 0770 }
+{ unix_sock_admin_perms = 0700 }
{ unix_sock_dir = /var/run/libvirt }
{ auth_unix_ro = none }
{ auth_unix_rw = none }
diff --git a/tests/confdata/libvirtd.conf b/tests/confdata/libvirtd.conf
index 2f2ba4b..5029c4c 100644
--- a/tests/confdata/libvirtd.conf
+++ b/tests/confdata/libvirtd.conf
@@ -89,6 +89,12 @@ unix_sock_ro_perms = 0777
# control then you may want to relax this to:
unix_sock_rw_perms = 0770
+# Set the UNIX socket permissions for the admin interface socket.
+#
+# Default allows only owner (root), do not change it unless you are
+# sure to whom you are exposing the access to
+unix_sock_admin_perms = 0700
+
#
diff --git a/tests/confdata/libvirtd.out b/tests/confdata/libvirtd.out
index 171945d..4d7ed47 100644
--- a/tests/confdata/libvirtd.out
+++ b/tests/confdata/libvirtd.out
@@ -71,6 +71,11 @@ unix_sock_ro_perms = 0777
# If not using PolicyKit and setting group ownership for access
# control then you may want to relax this to:
unix_sock_rw_perms = 0770
+# Set the UNIX socket permissions for the admin interface socket.
+#
+# Default allows only owner (root), do not change it unless you are
+# sure to whom you are exposing the access to
+unix_sock_admin_perms = 0700
#
#
# Authentication.
--
2.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/15] Build client headers for admin protocol
Signed-off-by: Martin Kletzander mklet...@redhat.com --- .gitignore | 1 + cfg.mk | 5 - src/Makefile.am | 9 - 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 56916cf..0f8b3d6 100644 --- a/.gitignore +++ b/.gitignore @@ -110,6 +110,7 @@ /src/access/viraccessapichecklxc.h /src/access/viraccessapicheckqemu.c /src/access/viraccessapicheckqemu.h +/src/admin/admin_client.h /src/admin/admin_protocol.[ch] /src/esx/*.generated.* /src/hyperv/*.generated.* diff --git a/cfg.mk b/cfg.mk index 09803e4..8f20f9b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1070,13 +1070,16 @@ bracket-spacing-check: sc_po_check: \ $(srcdir)/daemon/remote_dispatch.h \ $(srcdir)/daemon/qemu_dispatch.h \ - $(srcdir)/src/remote/remote_client_bodies.h + $(srcdir)/src/remote/remote_client_bodies.h \ + $(srcdir)/src/admin/admin_client.h $(srcdir)/daemon/remote_dispatch.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C daemon remote_dispatch.h $(srcdir)/daemon/qemu_dispatch.h: $(srcdir)/src/remote/qemu_protocol.x $(MAKE) -C daemon qemu_dispatch.h $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C src remote/remote_client_bodies.h +$(srcdir)/src/admin/admin_client.h: $(srcdir)/src/admin/admin_protocol.x + $(MAKE) -C src admin/admin_client.h # List all syntax-check exemptions: exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.h$$ diff --git a/src/Makefile.am b/src/Makefile.am index b1044a7..6813554 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -397,7 +397,14 @@ ADMIN_PROTOCOL = $(srcdir)/admin/admin_protocol.x ADMIN_PROTOCOL_GENERATED = \ admin/admin_protocol.c \ - admin/admin_protocol.h + admin/admin_protocol.h \ + admin/admin_client.h + +admin/admin_client.h: $(srcdir)/rpc/gendispatch.pl \ + $(ADMIN_PROTOCOL) Makefile.am + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=client \ + admin ADMIN $(ADMIN_PROTOCOL) \ + $(srcdir)/admin/admin_client.h EXTRA_DIST += $(ADMIN_PROTOCOL) $(ADMIN_PROTOCOL_GENERATED) BUILT_SOURCES += $(ADMIN_PROTOCOL_GENERATED) -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] lxc: move wireless PHYs to a network namespace
On 14.04.2015 18:21, Lubomir Rintel wrote:
The 802.11 interfaces can not be moved by themselves, their Phy has to move
too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
---
src/util/virnetdev.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index ee60f09..df48763 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -584,6 +584,8 @@ int virNetDevSetNamespace(const char *ifname, pid_t
pidInNs)
{
int rc;
char *pid = NULL;
+char *phy_path = NULL;
+
const char *argv[] = {
ip, link, set, ifname, netns, NULL, NULL
};
@@ -591,6 +593,35 @@ int virNetDevSetNamespace(const char *ifname, pid_t
pidInNs)
if (virAsprintf(pid, %lld, (long long) pidInNs) == -1)
return -1;
+/* The 802.11 wireless devices only move together with their PHY. */
+if (virNetDevSysfsFile(phy_path, ifname, phy80211/name) = 0) {
+int len;
+char *phy = NULL;
+
+len = virFileReadAllQuiet(phy_path, 1024, phy);
+VIR_FREE(phy_path);
+
+/* Remove a line break. */
+if (len 0)
+phy[len - 1] = '\0';
+
+if (len = 0) {
+const char *iwargv[] = {
+iw, phy, phy, set, netns, NULL, NULL
+};
+
+iwargv[5] = pid;
+rc = virRun(iwargv, NULL);
+
+VIR_FREE(phy);
+
+if (rc == 0) {
+VIR_FREE(pid);
+return rc;
+}
+}
+}
+
argv[5] = pid;
rc = virRun(argv, NULL);
I understand what you mean, but the code style could be better. I'll
polish and resend the patch in your name.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: move wireless PHYs to a network namespace
From: Lubomir Rintel lkund...@v3.sk
The 802.11 interfaces can not be moved by themselves, their Phy has to move too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virnetdev.c | 39 ---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index a816e5d..21fa9d0 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -560,20 +560,53 @@ int virNetDevSetMTUFromDevice(const char *ifname,
*/
int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
{
-int rc;
+int ret = -1;
char *pid = NULL;
+char *phy = NULL;
+char *phy_path = NULL;
+int len;
+
const char *argv[] = {
ip, link, set, ifname, netns, NULL, NULL
};
+const char *iwargv[] = {
+iw, phy, NULL, set, netns, NULL, NULL
+};
+
if (virAsprintf(pid, %lld, (long long) pidInNs) == -1)
return -1;
argv[5] = pid;
-rc = virRun(argv, NULL);
+if (virRun(argv, NULL) 0)
+goto cleanup;
+/* The 802.11 wireless devices only move together with their PHY. */
+if (virNetDevSysfsFile(phy_path, ifname, phy80211/name) 0)
+goto cleanup;
+
+if ((len = virFileReadAllQuiet(phy_path, 1024, phy) 0)) {
+if (errno == ENOENT) {
+/* Okay, this is not a wireless card. Claim success. */
+ret = 0;
+}
+goto cleanup;
+}
+
+/* Remove a line break. */
+phy[len - 1] = '\0';
+
+iwargv[2] = phy;
+iwargv[5] = pid;
+if (virRun(iwargv, NULL) 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(phy_path);
+VIR_FREE(phy);
VIR_FREE(pid);
-return rc;
+return ret;
}
#if defined(SIOCSIFNAME) defined(HAVE_STRUCT_IFREQ)
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetSocketNewConnectUNIX: Don't unlink(NULL)
On 04/16/2015 10:28 AM, Michal Privoznik wrote: There is a possibility that we jump onto error label with @lockpath still initialized to NULL. Here, the @lockpath should be unlink()-ed, but passing there a NULL is not a good idea. Don't do that. In fact, we should call unlink() only if we created the lock file successfully. Reported-by: John Ferlan jfer...@redhat.com Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/rpc/virnetsocket.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) ACK (Coverity is happy too) John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/7] virNetDevBandwidthSet: Add priority to filter
On 04/14/2015 12:59 PM, Michal Privoznik wrote: Currently, when constructing traffic shaping rules, the ingress filter is created without any priority specified on the command line. This makes kernel to make up one. While this works, it simplifies things a big if we provide the filter priority. In s/big/bit/ this case, since it's the root filter lets have it the highest s/have/give/ priority of number 1. Signed-off-by: Michal Privoznik mpriv...@redhat.com ACK. --- src/util/virnetdevbandwidth.c | 4 ++-- tests/virnetdevbandwidthtest.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index d1c0f12..943178b 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -209,8 +209,8 @@ virNetDevBandwidthSet(const char *ifname, virCommandFree(cmd); cmd = virCommandNew(TC); virCommandAddArgList(cmd, filter, add, dev, ifname, parent, - 1:0, protocol, all, handle, 1, fw, - flowid, 1, NULL); + 1:0, protocol, all, prio, 1, handle, + 1, fw, flowid, 1, NULL); if (virCommandRun(cmd, NULL) 0) goto cleanup; diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 3b46455..5a3f02c 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -139,7 +139,7 @@ mymain(void) TC qdisc add dev eth0 root handle 1: htb default 1\n TC class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps\n TC qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n - TC filter add dev eth0 parent 1:0 protocol all handle 1 fw flowid 1\n)); + TC filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw flowid 1\n)); DO_TEST_SET((bandwidth outbound average='1024'/ @@ -159,7 +159,7 @@ mymain(void) TC qdisc add dev eth0 root handle 1: htb default 1\n TC class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil 2kbps burst 4kb\n TC qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n - TC filter add dev eth0 parent 1:0 protocol all handle 1 fw flowid 1\n + TC filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw flowid 1\n TC qdisc add dev eth0 ingress\n TC filter add dev eth0 parent : protocol all u32 match u32 0 0 police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n)); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] virDomainActualNetDefContentsFormat: Format class_id more frequently
On 04/14/2015 12:59 PM, Michal Privoznik wrote:
After a360912179 the formatting of virDomainActualNetDefPtr was
changed a bit. However, during the function rewrite, iface's class_id
is not formatted as frequently as it could be. In fact, after rewrite
it's formatted only for iface of type VIR_DOMAIN_NET_TYPE_DIRECT where
it makes no sense and is unused. While where needed (_TYPE_NETWORK) is
not formatted at all.
Yikes! That was a typo when moving the code around. Should have been
VIR_DOMAIN_NET_TYPE_NETWORK (that bit was inside case
VIR_DOMAIN_NET_TYPE_NETWORK previous to the patch)
This makes the daemon forget it upon daemon
restart resulting in bad behaviour.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/domain_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4d7e3c9..ab4f2bf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18608,8 +18608,7 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
virBufferAddLit(buf, /\n);
}
-if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT
-def-data.network.actual def-data.network.actual-class_id) {
+if (def-data.network.actual def-data.network.actual-class_id) {
virBufferAsprintf(buf, class id='%u'/\n,
def-data.network.actual-class_id);
}
I suppose since the modes that wouldn't have been able to setup any
bandwidth limiting would have class_id == 0, this works. If you for some
reason wanted to protect against using it for hostdev interfaces, yoou
could move it up into the preceding else clause, but I guess I don't see
any gain from that.
ACK.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc: move wireless PHYs to a network namespace
On Thu, Apr 16, 2015 at 05:22:55PM +0200, Michal Privoznik wrote:
From: Lubomir Rintel lkund...@v3.sk
The 802.11 interfaces can not be moved by themselves, their Phy has to move
too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virnetdev.c | 39 ---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index a816e5d..21fa9d0 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -560,20 +560,53 @@ int virNetDevSetMTUFromDevice(const char *ifname,
*/
int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
{
-int rc;
+int ret = -1;
char *pid = NULL;
+char *phy = NULL;
+char *phy_path = NULL;
+int len;
+
const char *argv[] = {
ip, link, set, ifname, netns, NULL, NULL
};
+const char *iwargv[] = {
+iw, phy, NULL, set, netns, NULL, NULL
+};
+
if (virAsprintf(pid, %lld, (long long) pidInNs) == -1)
return -1;
argv[5] = pid;
-rc = virRun(argv, NULL);
+if (virRun(argv, NULL) 0)
+goto cleanup;
+/* The 802.11 wireless devices only move together with their PHY. */
+if (virNetDevSysfsFile(phy_path, ifname, phy80211/name) 0)
+goto cleanup;
+
+if ((len = virFileReadAllQuiet(phy_path, 1024, phy) 0)) {
+if (errno == ENOENT) {
+/* Okay, this is not a wireless card. Claim success. */
+ret = 0;
+}
+goto cleanup;
+}
+
+/* Remove a line break. */
+phy[len - 1] = '\0';
+
+iwargv[2] = phy;
+iwargv[5] = pid;
+if (virRun(iwargv, NULL) 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(phy_path);
+VIR_FREE(phy);
VIR_FREE(pid);
-return rc;
+return ret;
}
#if defined(SIOCSIFNAME) defined(HAVE_STRUCT_IFREQ)
ACK, this matches what lxc tools do.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Partly initialize driver even for non-privileged users
On 04/16/2015 04:45 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1211436
This reverts commit b7829f959b33c6e3242a9ed745c0da7dc696.
Well, not exactly - it adds something too :-)
The previous fix was not correct. Like everywhere else, a driver is a
global variable allocated in stateInitialize function (or something
similar for stateless drivers). Later, when a driver API is called,
it's possible that the global variable is accessed and dereferenced.
Now, some drivers require root privileges because they undertake some
actions reserved only for the system admin (e.g. manipulating host
firewall). And here's the trouble, the NWFilter state initializer
exited too early when finding out it's running unprivileged, leaving
the global NWFilter driver variable uninitialized. Any subsequent
API call that tried to lock the driver resulted in dereferencing the
driver and thus crash.
On the other hand, in order to not resurrect the bug the original
commit was fixing, Let's forbid the nwfilter define in session mode.
Sure, makes sense. The other APIs will just return nothing found since
there are no existing filters when the driver is started and none can be
defined.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Conflicts:
src/nwfilter/nwfilter_driver.c: Context. Code changed a bit
since 2013.
I don't know how useful this info is, since it's not an exact revert anyway.
ACK.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/nwfilter/nwfilter_driver.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 8e3db43..1a868b6 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -176,9 +176,6 @@ nwfilterStateInitialize(bool privileged,
char *base = NULL;
DBusConnection *sysbus = NULL;
-if (!privileged)
-return 0;
-
if (virDBusHasSystemBus()
!(sysbus = virDBusGetSystemBus()))
return -1;
@@ -193,6 +190,9 @@ nwfilterStateInitialize(bool privileged,
driver-watchingFirewallD = (sysbus != NULL);
driver-privileged = privileged;
+if (!privileged)
+return 0;
+
nwfilterDriverLock();
if (virNWFilterIPAddrMapInit() 0)
@@ -535,6 +535,12 @@ nwfilterDefineXML(virConnectPtr conn,
virNWFilterObjPtr nwfilter = NULL;
virNWFilterPtr ret = NULL;
+if (!driver-privileged) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Can't define NWFilters in session mode));
+return NULL;
+}
+
nwfilterDriverLock();
virNWFilterWriteLockFilterUpdates();
virNWFilterCallbackDriversLock();
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] refactor virStorageBackendSCSINewLun
On 04/16/2015 08:35 AM, Ján Tomko wrote: On Tue, Apr 07, 2015 at 04:21:01PM -0400, John Ferlan wrote: This could use some comment about inverting the logic of the retval variable. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 55 ++ 1 file changed, 20 insertions(+), 35 deletions(-) ACK Jan Weird - usually my verbosity gets flagged as being too much, but not like me to have no commit message... In any case, adjusted commit message and pushed. Working through other comments and will post a v4 later today. Tks John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Re: libvirtd live-locking on CTX_LOCK when doing 'virsh domid save /tmp/blah' with guest corrupting memory (on purpose).
On 04/14/2015 11:31 AM, Ian Jackson wrote: Konrad Rzeszutek Wilk writes (libvirtd live-locking on CTX_LOCK when doing 'virsh domid save /tmp/blah' with guest corrupting memory (on purpose).): It looks like thread #10 is blocking in libxl_read_exactly waiting for 'libxl-save-helper'. Said application (see below) has dispatched an message through helper_getreply and is blocking on __read_nocancel. This is not supposed to block. helper_stdout_readable assumes that the fd is actually readable. However, for complicated reasons it can happen in a multithreaded program that the fd was _reviously_ readable and is now no longer. This was not clearly documented in the internal API documentation. I have produced what I think are two patches that will fix this. I have compiled them but I haven't tested them. Konrad, are you able to check whether they fix your bug ? I too saw this bug just before Konrad's report, but the patches don't seem to help. Running a script that continually saves and restores domains will eventually lock libvirtd with essentially the same traces reported by Konrad Thread 4 (Thread 0x7fffee3a0700 (LWP 39068)): #0 0x73a9aa9d in read () from /lib64/libpthread.so.0 #1 0x74540ea0 in libxl_read_exactly (ctx=0x7fffe00445e0, fd=37, data=0x7fffee39f36e, sz=2, source=0x7fffc80010c0 domain 6 save/restore helper stdout pipe, what=0x7458112a ipc msg header) at libxl_utils.c:430 #2 0x7454913a in helper_stdout_readable (egc=0x7fffee39f540, ev=0x7fffc8002038, fd=37, events=3, revents=1) at libxl_save_callout.c:281 #3 0x7454fafb in afterpoll_internal (egc=0x7fffee39f540, poller=0x7fffea00, nfds=4, fds=0x7fffe930, now=...) at libxl_event.c:1185 #4 0x7455127a in eventloop_iteration (egc=0x7fffee39f540, poller=0x7fffea00) at libxl_event.c:1645 #5 0x74551df1 in libxl__ao_inprogress (ao=0x7fffc8001060, file=0x74575e1b libxl.c, line=982, func=0x74578750 __func__.17561 libxl_domain_suspend) at libxl_event.c:1896 #6 0x7450e051 in libxl_domain_suspend (ctx=0x7fffe00445e0, domid=6, fd=29, flags=0, ao_how=0x0) at libxl.c:982 #7 0x7fffe8774636 in libxlDoDomainSave (driver=0x7fffe011f1c0, vm=0x7fffe004f950, to=0x7fffc8000990 /tmp/sles12gm-pv.img) at libxl/libxl_driver.c:1584 #8 0x7fffe8774a35 in libxlDomainSaveFlags (dom=0x7fffc8000de0, to=0x7fffc8000990 /tmp/sles12gm-pv.img, dxml=0x0, flags=0) at libxl/libxl_driver.c:1653 #9 0x7fffe8774b11 in libxlDomainSave (dom=0x7fffc8000de0, to=0x7fffc8000990 /tmp/sles12gm-pv.img) at libxl/libxl_driver.c:1678 #10 0x7751db15 in virDomainSave (domain=0x7fffc8000de0, to=0x7fffc80009d0 /tmp/sles12gm-pv.img) at libvirt-domain.c:839 ... Thread 1 (Thread 0x77fc18c0 (LWP 39059)): #0 0x73a9a7bc in __lll_lock_wait () from /lib64/libpthread.so.0 #1 0x73a964a4 in _L_lock_952 () from /lib64/libpthread.so.0 #2 0x73a96306 in pthread_mutex_lock () from /lib64/libpthread.so.0 #3 0x7454caf6 in libxl__ctx_lock (ctx=0x7fffe00445e0) at libxl_internal.h:3268 #4 0x7454fe98 in libxl_osevent_occurred_fd (ctx=0x7fffe00445e0, for_libxl=0x7fffe004f210, fd=32, events_ign=0, revents_ign=1) at libxl_event.c:1242 #5 0x7fffe8770573 in libxlFDEventCallback (watch=24, fd=32, vir_events=1, fd_info=0x55896c60) at libxl/libxl_driver.c:123 #6 0x773f71bc in virEventPollDispatchHandles (nfds=14, fds=0x55897fa0) at util/vireventpoll.c:508 #7 0x773f79f9 in virEventPollRunOnce () at util/vireventpoll.c:657 #8 0x773f58fa in virEventRunDefaultImpl () at util/virevent.c:308 #9 0x555c2131 in virNetServerRun (srv=0x55889980) at rpc/virnetserver.c:1139 #10 0x5556cf88 in main (argc=2, argv=0x7fffe378) at libvirtd.c:1489 Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virbuffer: fix build on rhel-6
On rhel-6 is broken gcc that reports this warning: util/virbuffer.c:500: error: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] Move the pragma directive before function virBufferEscapeString because since commit aeb5262e this function uses 'strchr' too. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- Pushed as build-breaker fix. src/util/virbuffer.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 3d13c90..5b338f8 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -421,6 +421,15 @@ virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr) buf-use += count; } +/* Work around spurious strchr() diagnostics given by -Wlogical-op + * for gcc 4.6. Doing it via a local pragma keeps the damage + * smaller than disabling it on the package level. Unfortunately, the + * affected GCCs don't allow diagnostic push/pop which would have + * further reduced the impact. */ +#if BROKEN_GCC_WLOGICALOP +# pragma GCC diagnostic ignored -Wlogical-op +#endif + /** * virBufferEscapeString: * @buf: the buffer to append to @@ -535,15 +544,6 @@ virBufferEscapeSexpr(virBufferPtr buf, virBufferEscape(buf, '\\', \\', format, str); } -/* Work around spurious strchr() diagnostics given by -Wlogical-op - * for gcc 4.6. Doing it via a local pragma keeps the damage - * smaller than disabling it on the package level. Unfortunately, the - * affected GCCs don't allow diagnostic push/pop which would have - * further reduced the impact. */ -#if BROKEN_GCC_WLOGICALOP -# pragma GCC diagnostic ignored -Wlogical-op -#endif - /** * virBufferEscape: * @buf: the buffer to append to -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virCondWaitUntil: calculate timespec correctly
On Thu, Apr 16, 2015 at 07:27:36PM +1000, Michael Chapman wrote:
ts.tv_nsec was off by a factor of 1000, making timeouts less than a
second in the future often expiring immediately.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/util/virthread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Wow, we had a bug like that there since... The introduction of that
function in commit e40438fa. That's more than 5 years.
Good catch, ACK, I'll push this shortly.
diff --git a/src/util/virthread.c b/src/util/virthread.c
index c2a9e7f..6c49515 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -164,7 +164,7 @@ int virCondWaitUntil(virCondPtr c, virMutexPtr m, unsigned
long long whenms)
struct timespec ts;
ts.tv_sec = whenms / 1000;
-ts.tv_nsec = (whenms % 1000) * 1000;
+ts.tv_nsec = (whenms % 1000) * 100;
if ((ret = pthread_cond_timedwait(c-cond, m-lock, ts)) != 0) {
errno = ret;
--
2.1.0
--
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 v3 3/4] scsi: Adjust return value for virStorageBackendSCSINewLun
On Tue, Apr 07, 2015 at 04:21:02PM -0400, John Ferlan wrote: Use virStorageBackendPoolUseDevPath API to determine whether creation of stable target path is possible for the volume. If not, then return failure. This will differentiate a failed virStorageBackendStablePath which won't need to be fatal. Thus, we'll add a -2 return value to differentiate that the failure was a result of either the inability to find the symlink for the device or failure to open the target path directory Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index b96caec..d3c6470 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -146,6 +146,16 @@ virStorageBackendSCSISerial(const char *dev) } +/* + * Attempt to create a new LUN + * + * Returns: + * + * 0 = Success + * -1 = Failure due to some sort of OOM or other fatal issue found when + *attempting to get/update information about a found volume + * -2 = Failure to find a stable path, not fatal, caller can try another + */ static int virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, uint32_t host ATTRIBUTE_UNUSED, @@ -158,6 +168,18 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, char *devpath = NULL; int retval = -1; +/* Before we get too far - let's see if the pool is using target path + * starting with /dev. Attempts to find a stable path not based on a + * pool target starting with /dev will fail and do lots of unnecessary + * work - so we'll short circuit here. This also rejects the non-stable path '/dev' that was accepted before. 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 v4 0/9] qemu: Add quorum support to libvirt
On 03/17/2015 03:25 PM, Matthias Gatto wrote: The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html Before we reach a year waiting on this and so we make some progress... I started looking at the series and naturally found conflicts with the patches. Not to be deterred I started with the first 5 patches since I figured they were mostly setup and not functional change. Of those patch 2 had the most conflict. Once resolved, patches 3-5 applied without issue... Patch 6, more conflicts, so I just focused on 1-5... Patch 2 had numerous conflicts in virStorageBackendCreateQemuImgCmd due to commit id fbcf7da95b872ac45cabc4356fc9c06e809d0237. After applying patch 5, I cscope searched on -backingStore in order to find any 'new' references that might have crept in (ignoring any backingStoreRaw references) and found: f9ea3d60119e82c02c00fbf3678c3ed20634dea1 (qemu_monitor_json.c) which I adjusted patch 2 in order to keep it in line. Beyond that in patch 2: * In storage_conf.c/*VolDefFormat() - I changed the (def-target) to just def-target which was mostly a consistency thing. I found a couple of others as well and adjusted them. I don't think it matters, since none of the references were (x-y)-z * In storage_backend_fs.c/virStorageBackendProbeTarget() - you created a *local* backingStore reference and then used that for an assignment, but didn't update target-backingStore in at least the first case/instance, so since we're not updating the setting in this pass, I adjusted the code as follows: -if (!(target-backingStore = virStorageSourceNewFromBacking(meta))) +backingStore = virStorageSourceGetBackingStore(target, 0); +if (!(backingStore = virStorageSourceNewFromBacking(meta))) back to setting target-backingStore in the if statement, then fetching after the if statement into the local variable: -backingStore = virStorageSourceGetBackingStore(target, 0); -if (!(backingStore = virStorageSourceNewFromBacking(meta))) +if (!(target-backingStore = virStorageSourceNewFromBacking(meta))) goto cleanup; +backingStore = virStorageSourceGetBackingStore(target, 0); I believe the virStorageSourceFree(backingStore) that follows will be OK since you'd be replacing it anyway in the if statement. I'll post my changes as a reply to patch 2 shortly. Of course changing the storage_backend_fs.c in patch 2 caused a ripple effect (like I expected) in patch 4, but no changes were necessary since patch 4 did the set and I had added the fetch after the set as part of my patch 2 adjustment. In patch 3, you have a 'bool' function returning a pointer value (which can either be something or NULL. I'm changing that to return !!src-backingStore;, which is what I believe you're really trying to return here. There's a couple places where it's checked, but luckily so far nothing happens with those checks and from what I read... * virDomainDiskBackingStoreParse - Code has already dereferenced backingStore so we know it won't return false * virStorageBackendProbeTarget - virStorageSourceNewFromBacking fetch/failure would be the same * virStorageBackendLogicalMakeVol - If backingStore was NULL we would have failed earlier * virStorageSourceCopy - virStorageSourceCopy fetch/failure would be the same As long as you think that looks good - I can push patches 1-4. While Patch 5 did apply cleanly, there are some issues with it. In patch 3 it's returning true/false and failures in virStorageBackendProbeTarget and virStorageSourceCopy would return some message as to why it failed (most likely memory allocation failures) which then propagate back to the caller to get a reason for the failure. With your change to patch 5, you're returning true/false only without always setting the reason (eg virReportError). That reason needs to be set so failure reason can be propagated back to the caller instead of the infamous failed for some reason. Additionally in all the places that don't check the return, you should add a ignore_value() around them to signify the code doesn't care (or maybe check those places to see if that don't care assumption is true). Once 5 is reworked, you'll have to rework patches 6-9 and repost since that's where the real/new functionality starts. Tks, John This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to
Re: [libvirt] [PATCH v4 3/9] virstoragefile: Add virStorageSourceSetBackingStore
From 052a5b1a87c090093fda1d6591fb4d37a1cb8c3c Mon Sep 17 00:00:00 2001
From: Matthias Gatto matthias.ga...@outscale.com
Date: Tue, 17 Mar 2015 20:25:36 +0100
Subject: [PATCH 3/5] virstoragefile: Add virStorageSourceSetBackingStore
As explained for virStorageSourceGetBackingStore, quorum allows
multiple backing store, this make the operation to set bs complex
because we have to check if the backingStore is used as an array
or a pointer, and set it differently in both case.
In order to help the manipulation of backing store, I've added a
function virStorageSourceSetBackingStore.
For now virStorageSourceSetBackingStore don't handle the case where
we have more than one backing store in virStorageSource.
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 10 ++
src/util/virstoragefile.h | 3 +++
3 files changed, 14 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f3f956e..aa14f19 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2125,6 +2125,7 @@ virStorageSourceParseRBDColonString;
virStorageSourcePoolDefFree;
virStorageSourcePoolModeTypeFromString;
virStorageSourcePoolModeTypeToString;
+virStorageSourceSetBackingStore;
virStorageTypeFromString;
virStorageTypeToString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index dcaf67b..ded02f3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1806,6 +1806,16 @@ virStorageSourceGetBackingStore(const virStorageSource *src,
}
+bool
+virStorageSourceSetBackingStore(virStorageSourcePtr src,
+virStorageSourcePtr backingStore,
+size_t pos ATTRIBUTE_UNUSED)
+{
+src-backingStore = backingStore;
+return !!src-backingStore;
+}
+
+
/**
* virStorageSourcePtr:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 4262b36..5f76324 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -289,6 +289,9 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+bool virStorageSourceSetBackingStore(virStorageSourcePtr src,
+virStorageSourcePtr backingStore,
+size_t pos);
virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src,
size_t pos);
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/9] virstoragefile: Always use virStorageSourceGetBackingStore to get backing store
From fe29212011d9a20e2f7a47de9c66572bfb55bc8e Mon Sep 17 00:00:00 2001
From: Matthias Gatto matthias.ga...@outscale.com
Date: Tue, 17 Mar 2015 20:25:35 +0100
Subject: [PATCH 2/5] virstoragefile: Always use
virStorageSourceGetBackingStore to get backing store
Uniformize backing store usage by calling virStorageSourceGetBackingStore
instead of setting backing store manually.
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/conf/domain_conf.c| 7 ---
src/conf/storage_conf.c | 6 +++---
src/qemu/qemu_cgroup.c| 4 ++--
src/qemu/qemu_domain.c| 2 +-
src/qemu/qemu_driver.c| 18 ++
src/qemu/qemu_monitor_json.c | 4 +++-
src/security/security_dac.c | 2 +-
src/security/security_selinux.c | 4 ++--
src/security/virt-aa-helper.c | 2 +-
src/storage/storage_backend.c | 20
src/storage/storage_backend_fs.c | 31 +--
src/storage/storage_backend_gluster.c | 8 +---
src/storage/storage_backend_logical.c | 10 ++
src/util/virstoragefile.c | 20 ++--
tests/virstoragetest.c| 14 +++---
15 files changed, 84 insertions(+), 68 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4d7e3c9..0800981 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17837,7 +17837,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
/* We currently don't output seclabels for backing chain element */
if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) 0 ||
virDomainDiskBackingStoreFormat(buf,
-backingStore-backingStore,
+virStorageSourceGetBackingStore(backingStore, 0),
backingStore-backingStoreRaw,
idx + 1) 0)
return -1;
@@ -17959,7 +17959,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
/* Don't format backingStore to inactive XMLs until the code for
* persistent storage of backing chains is ready. */
if (!(flags VIR_DOMAIN_DEF_FORMAT_INACTIVE)
-virDomainDiskBackingStoreFormat(buf, def-src-backingStore,
+virDomainDiskBackingStoreFormat(buf,
+virStorageSourceGetBackingStore(def-src, 0),
def-src-backingStoreRaw, 1) 0)
return -1;
@@ -22132,7 +22133,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
}
}
-for (tmp = disk-src; tmp; tmp = tmp-backingStore) {
+for (tmp = disk-src; tmp; tmp = virStorageSourceGetBackingStore(tmp, 0)) {
int actualType = virStorageSourceGetActualType(tmp);
/* execute the callback only for local storage */
if (actualType != VIR_STORAGE_TYPE_NETWORK
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 4852dfb..5781374 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1326,7 +1326,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
if (virStorageSize(unit, capacity, ret-target.capacity) 0)
goto error;
} else if (!(flags VIR_VOL_XML_PARSE_NO_CAPACITY)
- !((flags VIR_VOL_XML_PARSE_OPT_CAPACITY) ret-target.backingStore)) {
+ !((flags VIR_VOL_XML_PARSE_OPT_CAPACITY) virStorageSourceGetBackingStore(ret-target, 0))) {
virReportError(VIR_ERR_XML_ERROR, %s, _(missing capacity element));
goto error;
}
@@ -1635,9 +1635,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
def-target, target) 0)
goto cleanup;
-if (def-target.backingStore
+if (virStorageSourceGetBackingStore(def-target, 0)
virStorageVolTargetDefFormat(options, buf,
- def-target.backingStore,
+ virStorageSourceGetBackingStore(def-target, 0),
backingStore) 0)
goto cleanup;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e83342d..44363c9 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -121,7 +121,7 @@ qemuSetupDiskCgroup(virDomainObjPtr vm,
virStorageSourcePtr next;
bool forceReadonly = false;
-for (next = disk-src; next; next = next-backingStore) {
+for (next = disk-src; next; next = virStorageSourceGetBackingStore(next, 0)) {
if (qemuSetImageCgroupInternal(vm, next, false, forceReadonly) 0)
return -1;
@@ -139,7 +139,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm,
{
virStorageSourcePtr next;
-for (next = disk-src; next; next = next-backingStore) {
+for (next = disk-src; next; next =
[libvirt] [PATCH] virNetSocketNewConnectUNIX: Don't unlink(NULL)
There is a possibility that we jump onto error label with @lockpath
still initialized to NULL. Here, the @lockpath should be unlink()-ed,
but passing there a NULL is not a good idea. Don't do that. In fact,
we should call unlink() only if we created the lock file successfully.
Reported-by: John Ferlan jfer...@redhat.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/rpc/virnetsocket.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b824285..a59e3e1 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -622,7 +622,7 @@ int virNetSocketNewConnectUNIX(const char *path,
usleep(5000);
}
-if (lockfd) {
+if (lockfd != -1) {
unlink(lockpath);
VIR_FORCE_CLOSE(lockfd);
VIR_FREE(lockpath);
@@ -640,12 +640,13 @@ int virNetSocketNewConnectUNIX(const char *path,
return 0;
error:
-if (lockfd)
+if (lockfd != -1) {
unlink(lockpath);
+VIR_FORCE_CLOSE(lockfd);
+}
VIR_FREE(lockpath);
VIR_FREE(rundir);
VIR_FORCE_CLOSE(fd);
-VIR_FORCE_CLOSE(lockfd);
return -1;
}
#else
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Re: libvirtd live-locking on CTX_LOCK when doing 'virsh domid save /tmp/blah' with guest corrupting memory (on purpose).
Jim Fehlig writes (Re: [PATCH 0/2] Re: libvirtd live-locking on CTX_LOCK when doing 'virsh domid save /tmp/blah' with guest corrupting memory (on purpose).): On 04/14/2015 11:31 AM, Ian Jackson wrote: I have produced what I think are two patches that will fix this. I have compiled them but I haven't tested them. Konrad, are you able to check whether they fix your bug ? I too saw this bug just before Konrad's report, but the patches don't seem to help. Running a script that continually saves and restores domains will eventually lock libvirtd with essentially the same traces reported by Konrad I'm a total idiot. I do the recheck but I don't pay any attention to the result. I will send an updated approach which does this centrally. Ian. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/7] virnetdevbandwidth.c: Separate tc filter creation to a function
On 04/14/2015 12:59 PM, Michal Privoznik wrote:
Not only this simplifies the code a bit, it prepares the
environment for upcoming patches. The new
virNetDevBandwidthManipulateFilter() function is capable of both
removing a filter and adding a new one. At the same time! Yeah,
this is not currently used anywhere but look at the next commit
where you'll see it.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virnetdevbandwidth.c | 142
+++---
1 file changed, 106 insertions(+), 36 deletions(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 943178b..c57c8c0 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -43,6 +43,107 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
VIR_FREE(def);
}
+/**
+ * virNetDevBandwidthManipulateFilter:
+ * @ifname: interface to operate on
+ * @ifmac_ptr: MAC of the interface to create filter over
+ * @id: filter ID
+ * @class_id: where to place traffic
+ * @remove_old: whether to remove the filter
+ * @create_new: whether to create the filter
+ *
+ * TC filters are as crucial for traffic shaping as QDiscs. While
+ * QDisc acts like black boxes deciding which packets should be
s/QDisc acts/QDiscs act/
+ * held up and which should be sent immediately, it's filter who
s/filter who/the filter that/
+ * places a packet into the box. So, we may end up constructing a
+ * set of filters on a single device (e.g. a bridge) and filter
+ * the traffic into QDiscs based on the originating vNET device.
+ *
+ * Long story short, @ifname is the interface where the filter
+ * should be created. The @ifmac_ptr is the MAC address for which
+ * the filter should be created (usually different to the MAC
+ * address of @ifname). Then, like everything - even filters have
+ * an @id which should be unique (per @ifname). And @class_id
+ * tells into which QDisc should filter place the traffic.
I think this would be better as a bullet list of parameters with their
purposes, rather than a chummy paragraph of text :-)
+ *
+ * This function can be used for both, removing stale filter
+ * (@remove_old set to true) and creating new one (@create_new
+ * set to true). Both at once for the same price!
Same here. Not that I don't appreciate the nice tone of the
conversation, it's just easier to pick out what you need to know from a
list than from a paragraph.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise (with error reported).
+ */
+static int ATTRIBUTE_NONNULL(1)
+virNetDevBandwidthManipulateFilter(const char *ifname,
+ const virMacAddr *ifmac_ptr,
+ unsigned int id,
+ const char *class_id,
+ bool remove_old,
+ bool create_new)
How about making these two flags so that it will be easier to tell
what's intended when looking at a call to the function?
+{
+int ret = -1;
+char *filter_id = NULL;
+virCommandPtr cmd = NULL;
+unsigned char ifmac[VIR_MAC_BUFLEN];
+char *mac[2] = {NULL, NULL};
+
+if (!(remove_old || create_new)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(filter creation API error));
+goto cleanup;
+}
I'm never sure how far to go with this type of checking on args that are
only given literal values in internal-only functions (especially
static). Especially if it's static and only called from one or two
places, I tend to go on the light side. Every new log message is one
more thing to translate, and one more bit of space taken up...
ACK, modulo the above comments (any of which you can take or leave) and
assuming your functional testing showed it to produce identical results
to previous behavior (which I'm guessing is true, since you've just
found another bug that would have been noticed by testing bandwidth
management while restarting libvirtd :-)
+
+/* u32 filters must have 800:: prefix. Don't ask. */
+if (virAsprintf(filter_id, 800::%u, id) 0)
+goto cleanup;
+
+if (remove_old) {
+int cmd_ret = 0;
+
+cmd = virCommandNew(TC);
+virCommandAddArgList(cmd, filter, del, dev, ifname,
+ prio, 2, handle, filter_id, u32, NULL);
+
+if (virCommandRun(cmd, cmd_ret) 0)
+goto cleanup;
+
+}
+
+if (create_new) {
+virMacAddrGetRaw(ifmac_ptr, ifmac);
+
+if (virAsprintf(mac[0], 0x%02x%02x%02x%02x, ifmac[2],
+ifmac[3], ifmac[4], ifmac[5]) 0 ||
+virAsprintf(mac[1], 0x%02x%02x, ifmac[0], ifmac[1]) 0)
+goto cleanup;
+
+virCommandFree(cmd);
+cmd = virCommandNew(TC);
+/* Okay, this not nice. But since libvirt does not necessarily track
+ *
[libvirt] [PATCH] qemu: Always refresh capabilities if no guests found (bz 1000116)
- Remove all qemu emulators
- Restart libvirtd
- Install qemu emulators
- Call 'virsh version' - errors
The only thing that will force the qemu driver to refresh it's cached
capablities info is an explict API call to GetCapabilities.
However in the case when the initial caps lookup at driver connect didn't
find a single qemu emulator to poll, the driver is effectively useless
and really can't do anything until it's populated some qemu capabilities
info.
With the above steps, the user would have to either know about the
magic refresh capabilities call, or restart libvirtd to pick up the
changes.
Instead, this patch changes things so that every time a part of th
driver requests access to capabilities info, check to see if
we've previously seen any emulators. If not, force a refresh.
In the case of 'still no emulators found', this is still very quick, so
I can't think of a downside.
https://bugzilla.redhat.com/show_bug.cgi?id=1000116
---
src/qemu/qemu_conf.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2cf3905..b662b69 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -965,6 +965,13 @@ virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr
driver,
qemuDriverLock(driver);
}
+if (driver-caps-nguests == 0 !refresh) {
+VIR_DEBUG(Capabilities didn't detect any guests. Forcing a
+refresh.);
+qemuDriverUnlock(driver);
+return virQEMUDriverGetCapabilities(driver, true);
+}
+
ret = virObjectRef(driver-caps);
qemuDriverUnlock(driver);
return ret;
--
2.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: If installing default network, reload libvirtd (bz 867546)
If libvirt-daemon-config-network is installed while libvirtd is already
running, the daemon needs to be restarted to pick up the change.
Instead let's trigger a daemon reload when the package is first installed.
Then the default network is available immediately if libvirtd was already
running.
https://bugzilla.redhat.com/show_bug.cgi?id=867546
---
libvirt.spec.in | 8
1 file changed, 8 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index e08c9e7..ada0257 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1770,6 +1770,14 @@ if test $1 -eq 1 test ! -f
%{_sysconfdir}/libvirt/qemu/networks/default.xml ;
%{_datadir}/libvirt/networks/default.xml \
%{_sysconfdir}/libvirt/qemu/networks/default.xml
ln -s ../default.xml
%{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
+
+# Make sure libvirt picks up the new network defininiton
+ %if %{with_systemd}
+/bin/systemctl reload libvirtd.service /dev/null 21 ||:
+ %else
+/sbin/service libvirtd reload /dev/null 21 || :
+ %endif
+
fi
%endif
--
2.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: set MAC address for VF via netlink message to PF + VF# when possible
On 04/15/2015 02:47 PM, John Ferlan wrote:
On 04/10/2015 01:47 PM, Laine Stump wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1113474
When we set the MAC address of a network device as a part of setting
up macvtap passthrough mode (where the domain has an emulated netdev
connected to a host macvtap device that has exclusive use of the
physical device, and sets the device MAC address to match its own,
i.e. interface type='direct' source mode='passthrough' .../), we
use ioctl(SIOCSIFHWADDR) giving it the name of that device. This is
true even if it is an SRIOV Virtual Function (VF).
But, when we are setting the MAC address / vlan ID of a VF in
preparation for hostdev network passthrough (this is where we set
the MAC address and vlan id of the VF before detaching the host net
driver and assigning the device to the domain with PCI passthrough,
i.e. interface type='hostdev', we do the setting via a netlink
^^
Looks like a close ) is needed...
RTM_SETLINK message for that VF's Physical Function (PF), telling it
the VF# we want to change. This sets an administratively changed MAC
flag for that VF in the PF's driver, and from that point on (until the
PF's driver is reloaded) that VF's MAC address can't be changed.
This means that if a VF is used for hostdev passthrough, it will have
the admin flag set, and future attempts to use that VF for macvtap
passthrough will fail.
The solution to this problem is to check if a device being used for
macvtap passthrough is actually a VF; if so, we use the netlink
RTM_SETLINK message to the PF to set the VF's mac address instead of
ioctl(SIOCSIFHWADDR); if not, behavior does not change from previously.
There are three pieces to making this work:
1) virNetDevMacVLan(Create|Delete)WithVPortProfile() now call
virNetDev(Replace|Restore)NetConfig() rather than
virNetDev(Replace|Restore)MacAddress() (simply passing -1 for VF#
and vlanid).
2) virNetDev(Replace|Restore)NetConfig() check to see if the device is
a VF. If so, they find the PF's name and VF#, allowing them to call
virNetDev(Replace|Restore)VfConfig().
3) To prevent mixups when detaching a macvtap passthrough device that
had been attached while running an older version of libvirt,
virNetDevRestoreVfConfig() is potentially given the preserved name
of the VF, and if the proper statefile for a VF can't be found in
the stateDir (${stateDir}/${pfname}_vf${vfid}),
virNetDevRestoreMacAddress() is called instead (which will look in
the file named ${stateDir}/${vfname}).
This problem has existed in every version of libvirt that has both
macvtap passthrough and interface type='hostdev'. Fortunately people
seem to use one or the other though.
---
(I'm still doing some testing on this, but as long as this approach
doesn't cause any other regressions (so far it doesn't appear to),
this is the final form. So any ACK given will be contingent on the
functional tests passing.)
src/util/virnetdev.c| 66
-
src/util/virnetdevmacvlan.c | 4 +--
2 files changed, 62 insertions(+), 8 deletions(-)
Perhaps in order to avoid someone externally calling
virNetDevRestoreMacAddress and virNetDevReplaceMacAddress, they should
be come local/static to virnetdev.c thus forcing other module callers to
use virNetDevRestoreVfConfig or virNetDevReplaceNetConfig
Yeah, I thought about that, but vacillated about whether to make it part
of the same patch or a separate patch. Then as usually I forgot about
it. I think I'll just make them static as part of this patch.
Not a requirement, but it potentially avoids future issues...
In any case, ACK as long as your testing went well as your explanation
seems perfectly reasonable and succinct
Yep, everything worked out okay. I was unable to make macvtap
bridge/private mode work on the VF interfaces after this patch, but then
I went back to older libvirt and found that those modes don't work on
VFs *before* the patch either, and I don't think it's a regression, I
think it just never worked due to limitations about promiscuous mode on
VFs. In the end I think it's only practical to do those modes on the PF,
with the VF's useful for macvtap passthrough and direct device assignment.
I'll push this after making the abovementioned functions static.
Thanks for the review!
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Tue, 2015-04-14 at 20:15 -0600, Jim Fehlig wrote:
Konrad Rzeszutek Wilk wrote:
-static bool
-libxlDomainMigrationIsAllowed(virDomainDefPtr def)
-{
-/* Migration is not allowed if definition contains any hostdevs */
-if (def-nhostdevs 0) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain has assigned host devices));
-return false;
-}
-
-return true;
-}
-
This function was created with the intention of adding more checks, e.g.
is the cpu and numa configuration migratable?
Only vaguely related to the patch, I know (sorry!), but if I can ask,
what do you mean with is the cpu and numa configuration migratable?
I mean what NUMA configuration are we talking about, and when and under
what circumstances would you consider it migratable?
Thanks and Regards,
Dario
signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/4] scsi: Adjust return value for virStorageBackendSCSINewLun
On 04/16/2015 09:14 AM, Ján Tomko wrote:
On Tue, Apr 07, 2015 at 04:21:02PM -0400, John Ferlan wrote:
Use virStorageBackendPoolUseDevPath API to determine whether creation of
stable target path is possible for the volume. If not, then return failure.
This will differentiate a failed virStorageBackendStablePath which won't
need to be fatal. Thus, we'll add a -2 return value to differentiate that
the failure was a result of either the inability to find the symlink for
the device or failure to open the target path directory
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_scsi.c | 27 ---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index b96caec..d3c6470 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -146,6 +146,16 @@ virStorageBackendSCSISerial(const char *dev)
}
+/*
+ * Attempt to create a new LUN
+ *
+ * Returns:
+ *
+ * 0 = Success
+ * -1 = Failure due to some sort of OOM or other fatal issue found when
+ *attempting to get/update information about a found volume
+ * -2 = Failure to find a stable path, not fatal, caller can try another
+ */
static int
virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
uint32_t host ATTRIBUTE_UNUSED,
@@ -158,6 +168,18 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
char *devpath = NULL;
int retval = -1;
+/* Before we get too far - let's see if the pool is using target path
+ * starting with /dev. Attempts to find a stable path not based on a
+ * pool target starting with /dev will fail and do lots of unnecessary
+ * work - so we'll short circuit here.
This also rejects the non-stable path '/dev' that was accepted before.
Not quite sure I see the issue - can you be more specific? Am I missing
something obvious?
Previously if pool-def-target.path is /dev (or NULL or /dev/ or
didn't start with /dev), then we'd return a strdup'd 'devpath' into
'vol-target.path'.
I see no way 'devpath' could be '/dev' or '/dev/' since it is formatted as :
if (virAsprintf(devpath, /dev/%s, dev) 0)
goto cleanup;
where 'dev' is an argument to this function as found by processLU and
cannot be NULL from a call:
if (getBlockDevice(host, bus, target, lun, block_device) 0) {
VIR_DEBUG(Failed to find block device for this LUN);
goto out;
which fills 'block_device' from either getNewStyleBlockDevice or
getOldStyleBlockDevice.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/4] scsi: Adjust return values from processLU
On 04/16/2015 10:06 AM, Ján Tomko wrote:
On Tue, Apr 07, 2015 at 04:21:03PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1171933
Adjust the processLU error returns to be a bit more logical. Currently,
the calling code cannot determine the difference between a non disk/lun
volume and a processed/found disk/lun. It can also not differentiate
between perhaps real/fatal error and one that won't necessarily stop
the code from finding other volumes.
After this patch virStorageBackendSCSIFindLUsInternal will stop processing
as soon as a fatal message occurs rather than continuting processing
for no apparent reason. It will also only set the *found value when
at least one of the processLU's was successful.
With the failed return, if the reason for the stop was that the pool
target path did not exist, was /dev, was /dev/, or did not start with
/dev, then iSCSI pool startup and refresh will fail.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_scsi.c | 44
+++---
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index d3c6470..4367b9e 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -371,6 +371,16 @@ getBlockDevice(uint32_t host,
}
+/*
+ * Process a Logical Unit entry from the scsi host device directory
+ *
+ * Returns:
+ *
+ * 0 = Found a valid entry
Maybe 1 = found, 0 = not found, but no error?
We can choose whatever numbers we want as long as
virStorageBackendSCSIFindLUsInternal actually uses them.
I think we've been using -2 in some places lately as kind a non-fatal
marker of sorts and -1 as a fatal error.
I see no reason to change it here.
+ * -1 = Some sort of fatal error
+ * -2 = A non-fatal error such as a non disk/lun entry or an entry
Why the quotes?
Why not? I'm fine with your phraseology below and will use it.
Also, non-disk/cdrom isn't an error.
If we return -2 for that case as well, I'd phrase this as:
non-fatal error or a non-disk entry.
+ *without a block device found
+ */
static int
processLU(virStoragePoolObjPtr pool,
uint32_t host,
@@ -389,7 +399,7 @@ processLU(virStoragePoolObjPtr pool,
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to determine if %u:%u:%u:%u is a
Direct-Access LUN),
host, bus, target, lun);
-goto out;
+return -1;
}
/* We don't create volumes for devices other than disk and cdrom
@@ -397,32 +407,25 @@ processLU(virStoragePoolObjPtr pool,
* isn't an error, either. */
if (!(device_type == VIR_STORAGE_DEVICE_TYPE_DISK ||
device_type == VIR_STORAGE_DEVICE_TYPE_ROM))
-{
-retval = 0;
-goto out;
-}
+return -2;
VIR_DEBUG(%u:%u:%u:%u is a Direct-Access LUN,
host, bus, target, lun);
if (getBlockDevice(host, bus, target, lun, block_device) 0) {
VIR_DEBUG(Failed to find block device for this LUN);
-goto out;
+return -2;
Shouldn't this be fatal?
I suppose the VIR_DEBUG threw me off, but if that fails, then
block_device isn't generated for some real reason (memory, opendir
fail, ReadDir fail, strrchr fail) - probably not something we want to
continue from.
}
-if (virStorageBackendSCSINewLun(pool,
-host, bus, target, lun,
-block_device) 0) {
+retval = virStorageBackendSCSINewLun(pool, host, bus, target, lun,
+ block_device);
+if (retval 0)
VIR_DEBUG(Failed to create new storage volume for %u:%u:%u:%u,
host, bus, target, lun);
-goto out;
-}
-retval = 0;
-
-VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully,
- host, bus, target, lun);
+else
+VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully,
+ host, bus, target, lun);
I prefer the original logic where the success path is unindented:
if (ret 0) {
VIR_DEBUG(failure...);
goto cleanup;
}
ret = 0;
VIR_DEBUG(success)
Fine 6 of one, half dozen of another
- out:
VIR_FREE(block_device);
return retval;
}
@@ -456,6 +459,8 @@
virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr pool,
*found = false;
while ((retval = virDirRead(devicedir, lun_dirent, device_path)) 0) {
+int rc;
+
A 'rc' variable separated from the 'ret' value of the function could be
used for the virDirRead as well
virDirRead will return -1 on error which is what we want to return to
the caller.
I see no reason to change as we'd only then have to assign retval to
whatever value we invent - yet another
Re: [libvirt] [PATCH 0/5] libxl: improve xl config parsing
On 04/09/2015 08:01 AM, Michal Privoznik wrote:
On 20.03.2015 18:07, Jim Fehlig wrote:
This series was inspired by Marek's and Chunyan's patches to add support
for kernel, initrd, and cmdline in Xen HVM domain config
https://www.redhat.com/archives/libvir-list/2015-March/msg00328.html
https://www.redhat.com/archives/libvir-list/2014-September/msg01006.html
Patches 1 and 2 are trivial prep for patch 3, which moves parsing and
formatting of os config out of the common code and into the xl and xm
specfic parsing/formatting code. For ease of review, identical copies
of xen{Parse,Format}OS are made in xen_{xl,xm}.c. Changes to the xl
parsing/formatting functions are made in patch 4 and 5.
Changing the order in which config is parsed/formatted has the
unfortunate side affect of requiring corresponding changes to the
test config data files. Patch 3 shoulders that burden.
Jim Fehlig (5):
xenconfig: export xenConfigCopyString
xenconfig: remove redunant parsing of device_model
xenconfig: move os parsing/formating to config-specific files
xenconfig: don't use kernel for hvmloader
libxl: support HVM direct kernel boot
src/libxl/libxl_conf.c | 9 ++
src/xenconfig/xen_common.c | 146 +
src/xenconfig/xen_common.h | 5 +
src/xenconfig/xen_xl.c | 178 +
src/xenconfig/xen_xm.c | 141
tests/testutilsxen.c | 8 +-
.../test-fullvirt-direct-kernel-boot.cfg | 29
.../test-fullvirt-direct-kernel-boot.xml | 49 ++
tests/xlconfigdata/test-fullvirt-multiusb.cfg | 5 +-
tests/xlconfigdata/test-new-disk.cfg | 5 +-
tests/xlconfigdata/test-spice.cfg | 5 +-
tests/xlconfigtest.c | 3 +
tests/xmconfigdata/test-escape-paths.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-force-hpet.cfg| 6 +-
tests/xmconfigdata/test-fullvirt-force-nohpet.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-localtime.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-net-ioemu.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-net-netfront.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-new-cdrom.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-old-cdrom.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-parallel-tcp.cfg | 6 +-
.../test-fullvirt-serial-dev-2-ports.cfg | 6 +-
.../test-fullvirt-serial-dev-2nd-port.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-serial-file.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-serial-null.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-serial-pipe.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-serial-pty.cfg| 6 +-
tests/xmconfigdata/test-fullvirt-serial-stdio.cfg | 6 +-
.../test-fullvirt-serial-tcp-telnet.cfg| 6 +-
tests/xmconfigdata/test-fullvirt-serial-tcp.cfg| 6 +-
tests/xmconfigdata/test-fullvirt-serial-udp.cfg| 6 +-
tests/xmconfigdata/test-fullvirt-serial-unix.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-sound.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 6 +-
tests/xmconfigdata/test-fullvirt-utc.cfg | 6 +-
tests/xmconfigdata/test-no-source-cdrom.cfg| 6 +-
tests/xmconfigdata/test-paravirt-net-e1000.cfg | 2 +-
tests/xmconfigdata/test-paravirt-net-vifname.cfg | 2 +-
.../test-paravirt-new-pvfb-vncdisplay.cfg | 2 +-
tests/xmconfigdata/test-paravirt-new-pvfb.cfg | 2 +-
.../test-paravirt-old-pvfb-vncdisplay.cfg | 2 +-
tests/xmconfigdata/test-paravirt-old-pvfb.cfg | 2 +-
tests/xmconfigdata/test-paravirt-vcpu.cfg | 2 +-
tests/xmconfigdata/test-pci-devs.cfg | 6 +-
45 files changed, 511 insertions(+), 242 deletions(-)
create mode 100644 tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml
ACK although you will have to rebase the last patch.
5/5 rebased and the series is pushed now. Thanks!
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add source check before attaching device
On 2015/4/16 17:44, Jiri Denemark wrote: On Thu, Apr 16, 2015 at 16:51:52 +0800, zhang bo wrote: Source device/file is not unique now, we should check it when attach device. The check is vary fragile and unreliable. It would be very hard (perhaps even impossible) to really detect that the two disks/filesystems point to the same source, which means libvirt would not really prevent anyone from attaching a single source twice. Moreover, the code as written would incorrectly refuse to attach disks that do not point to the same source (network disks, e.g.). But anyway, I don't think libvirt should be doing this in general, so NACK to this idea. Moreover, you should be able to achieve the same goal (and even more) by using a locking driver. Jirka That's true! we didn't think of that. Thank you Jirka. oscar -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] request for backwards compatibility
Hi there, I've been trying to use libvirt-php to bring libvirt support to jentu which runs on mostly Debian Wheezy machines. I've had a few bugs setting network devices as active with the released (0.4.8) version: PHP Warning: libvirt_network_get_information(): Invalid XPath node for network gateway IP address in /var/www/libjentu-virt.php on line 54 PHP Warning: libvirt_network_set_active() expects exactly 2 parameters, 1 given in /var/www/libjentu-virt.php on line 55 PHP Warning: libvirt_network_set_active(): Invalid arguments in /var/www/libjentu-virt.php on line 55 Building the latest Git repository results in: gcc -g -O2 -Wall -fpic -DCOMPILE_DL_LIBVIRT=1 -I/usr/include/php5 -I/usr/include/php5/main -I/usr/include/php5/TSRM -I/usr/include/php5/Zend -I/usr/include/php5/ext -I/usr/include/php5/ext/date/lib -c -o libvirt-php.o libvirt-php.c -I/usr/include/libxml2-DHAVE_CONFIG_H libvirt-php.c: In function ‘zm_startup_libvirt’: libvirt-php.c:1179:2: error: ‘VIR_DOMAIN_VCPU_GUEST’ undeclared (first use in this function) libvirt-php.c:1179:2: note: each undeclared identifier is reported only once for each function it appears in libvirt-php.c: In function ‘get_next_free_numeric_value’: libvirt-php.c:2719:6: warning: format ‘%x’ expects argument of type ‘unsigned int *’, but argument 3 has type ‘long int *’ [-Wformat] make[2]: *** [build] Error 1 And this is because debian wheezy has an old version of libvirt that does not have this declared. The proper solution looks like it'd be a configure-time check and enable or disable the new VCPU guest modifiers if they are not available because I'm not so sure wheezy will ever get a newer version of libvirt. Thanks, Kash Pande Jentu Technologies, Inc. http://jentu-networks.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: lifecycle: guest get rebooted after reboot and shutdown
Steps:
1 virsh reboot guest1 --mode=acpi
2 virsh shutdown guest1 --mode=agent
Expected result:
As the SHUTDOWN job is after REBOOT, we expected the guest to be *shutoff*.
Exacted result:
After the 2 steps above, the guest got *rebooted*.
The reason to this problem:
1 in qemuDomainReboot(mode acpi), it sets priv-fakeReboot to 1.
2 after shutdown/reboot, qemu monitor IO trigged
qemuProcessHandleShutdown(), which finds that priv-fakeReboot is 1, and reboot
the guest.
Root Cause of the problem:
After further look into the problem, We found that the design of
acpi/agent shutdown/reboot seems a little chaotic.
---
sheet1 who sets fakeReboot
---
shutdown reboot
acpiY(0) Y(1)
agent N N
It's apparently, *acpi-mode* jobs set fakeReboot.
---
sheet2 who needs to check fakeReboot(qemuProcessHandleShutdown())
---
shutdown reboot
acpiY Y
agent *Y* *N*
Things become a little odd here. only agent-mode reboot doesn't check
fakeReboot.
We can tell from the above 2 sheets, that they're not consistent.
*Agent-mode shutdown checks fakeReboot(trigger by SHUTDOWN monitorIO
event), while it didn't set it before.*
The chaos is not caused by libvirtd, it's a systematic problem, including
guest os and qemu. (guest os writes ACPI, triggers qemu, qemu then triggers
libvirtd)
Solution:
A simple solution is to make the 1st sheet consistent to the 2nd sheet,
which is:
---
sheet3 who should set fakeReboot:
---
shutdown reboot
acpiY(0) Y(1)
agent *Y(0)* N
---
we let agent-mode shutdown set fakeReboot to 0.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7eb5a7d..c751dcf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1959,6 +1959,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
goto endjob;
}
+qemuDomainSetFakeReboot(driver, vm, isReboot);
+
if (useAgent) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv-agent, agentFlag);
@@ -1970,7 +1972,6 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
*/
if (!useAgent ||
(ret 0 (acpiRequested || !flags))) {
-qemuDomainSetFakeReboot(driver, vm, isReboot);
/* Even if agent failed, we have to check if guest went away
* by itself while our locks were down. */
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3 0/3] libxl: domain destroy fixes
On 04/09/2015 09:03 AM, Michal Privoznik wrote: On 04.04.2015 00:49, Jim Fehlig wrote: 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(-) ACK series, but see my comment to the first patch. I fixed the comment for libxlDomainStart as you suggested and pushed the series. Thanks for the review Michal! Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] Introduce virNetDevBandwidthUpdateFilter
On 04/14/2015 12:59 PM, Michal Privoznik wrote:
This is practically a wrapper over
This is a simple wrapper around
virNetDevBandwidthManipulateFilter() that will update the desired
filter on an interface (usually a network bridge) with a new MAC
address. Although, the MAC address in question usually refers to
some other interface - the one that the filter is constructed
for. Yeah, hard to parse. Thing is, our NATed network has a
Again with the extra unnecessary verbiage :-)
bridge where some part of QoS takes place. And vNICs from guests
are plugged into the bridge. However, if a guest decides to
change the MAC of its vNIC, corresponding qemu process emits an
s/corresponding/the corresponding/
event to which we respond somehow.
... an event which we can use to update the QoS configuration based on
the new MAC address.
However, our QoS hierarchy is
currently not notified, therefore it falls apart.
falls apart is a bit lacking on the details :-)
This function
(when called from the correct place)
(when called in response to the aforementioned event)
will update our QoS
hierarchy and duck tape it together again.
Technically, it's duct tape (although there is one company that
capitalizes on the mispronunciation by calling theirs Duck(tm) Tape.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/virnetdevbandwidth.c | 38 ++
src/util/virnetdevbandwidth.h | 6 ++
3 files changed, 45 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7166283..547a919 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1753,6 +1753,7 @@ virNetDevBandwidthFree;
virNetDevBandwidthPlug;
virNetDevBandwidthSet;
virNetDevBandwidthUnplug;
+virNetDevBandwidthUpdateFilter;
virNetDevBandwidthUpdateRate;
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index c57c8c0..0c49b41 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -680,3 +680,41 @@ virNetDevBandwidthUpdateRate(const char *ifname,
VIR_FREE(ceil);
return ret;
}
+
+/**
+ * virNetDevBandwidthUpdateFilter:
+ * @ifname: interface to operate on
+ * @ifmac_ptr: new MAC to update the filter with
+ * @id: filter ID
+ *
+ * Sometimes the host environment is so dynamic, that even
+ * guest's MAC addresses change on the fly. That's when we must
s/guest's/a guest's/
s/That's when/When that happens/
+ * update our QoS hierarchy so that guest's traffic is placed
s/guest's/the guest's/
+ * into correct QDiscs.
s/correct/the correct/
This function does exactly that. On the
+ * interface @ifname (usually a bridge) updates filter with
+ * unique identifier @id so that it reflects fact, that new
+ * address corresponding to the filter has changed to @ifmac_ptr.
This function updates the filter for the interface @ifname with the
unique identifier @id so that it uses the new MAC address of the guest
interface @ifmac_ptr.
+ *
+ * Returns: 0 on success,
+ * -1 on failure (with error reported).
+ */
+int
+virNetDevBandwidthUpdateFilter(const char *ifname,
+ const virMacAddr *ifmac_ptr,
+ unsigned int id)
+{
+int ret = -1;
+char *class_id = NULL;
+
+if (virAsprintf(class_id, 1:%x, id) 0)
+goto cleanup;
+
+if (virNetDevBandwidthManipulateFilter(ifname, ifmac_ptr, id,
+ class_id, true, true) 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(class_id);
+return ret;
+}
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index efcf95a..9b1d2a6 100644
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -73,4 +73,10 @@ int virNetDevBandwidthUpdateRate(const char *ifname,
unsigned long long new_rate)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_RETURN_CHECK;
+
+int virNetDevBandwidthUpdateFilter(const char *ifname,
+ const virMacAddr *ifmac_ptr,
+ unsigned int id)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ATTRIBUTE_RETURN_CHECK;
#endif /* __VIR_NETDEV_BANDWIDTH_H__ */
ACK with the description cleaned up a bit.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
