Re: [libvirt] [PATCH V5 09/12] src/xenxs: Refactor code formating OS config
Kiarie Kahurani wrote:
introduce function
xenFormatXMOS(virConfPtr conf,);
I split this into three functions:
- xenFormatXMEmulator
- xenFormatXMCDROM
- xenFormatXMOS
Formating of emulator and cdrom can be different between xm and xl, and
we may want to account for that in the near future.
which formats OS config instead
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 95
--
tests/xmconfigdata/test-escape-paths.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-force-hpet.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-force-nohpet.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-localtime.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-net-ioemu.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-net-netfront.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-new-cdrom.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-old-cdrom.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-parallel-tcp.cfg | 2 +-
.../test-fullvirt-serial-dev-2-ports.cfg | 2 +-
.../test-fullvirt-serial-dev-2nd-port.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-file.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-null.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-pipe.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-pty.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-stdio.cfg | 2 +-
.../test-fullvirt-serial-tcp-telnet.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-tcp.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-udp.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-unix.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-sound.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-utc.cfg | 2 +-
tests/xmconfigdata/test-no-source-cdrom.cfg| 2 +-
tests/xmconfigdata/test-pci-devs.cfg | 2 +-
Avoids changing all the test data files too. Simplified patch below.
BTW, I got interrupted during my earlier review, so will push shortly
had changed to will push tomorrow :-).
Regards,
Jim
From 76824b192ecac2d9f19406c11446b616ca96e9d4 Mon Sep 17 00:00:00 2001
From: Kiarie Kahurani davidkiar...@gmail.com
Date: Tue, 12 Aug 2014 00:21:32 +0300
Subject: [PATCH 08/11] src/xenxs: Refactor code formating OS config
introduce functions
xenFormatXMEmulator(virConfPtr conf,);
xenFormatXMCDROM(virConfPtr conf, ...);
xenFormatXMOS(virConfPtr conf,);
which formats OS and associated config instead
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/xenxs/xen_xm.c | 161 +
1 file changed, 101 insertions(+), 60 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 432fee2..72ae913 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -2021,42 +2021,57 @@ xenFormatXMCPUFeatures(virConfPtr conf,
}
-/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
- either 32, or 64 on a platform where long is big enough. */
-verify(MAX_VIRT_CPUS = sizeof(1UL) * CHAR_BIT);
+static int
+xenFormatXMEmulator(virConfPtr conf, virDomainDefPtr def)
+{
+if (def-emulator
+xenXMConfigSetString(conf, device_model, def-emulator) 0)
+return -1;
-virConfPtr
-xenFormatXM(virConnectPtr conn,
-virDomainDefPtr def,
-int xendConfigVersion)
+return 0;
+}
+
+
+static int
+xenFormatXMCDROM(virConfPtr conf,
+ virDomainDefPtr def,
+ int xendConfigVersion)
{
-virConfPtr conf = NULL;
-int hvm = 0;
size_t i;
-virConfValuePtr netVal = NULL;
-
-if (!(conf = virConfNew()))
-goto cleanup;
-if (xenFormatXMGeneralMeta(conf, def) 0)
-goto cleanup;
+if (STREQ(def-os.type, hvm)) {
+if (xendConfigVersion == XEND_CONFIG_VERSION_3_0_2) {
+for (i = 0; i def-ndisks; i++) {
+if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_CDROM
+def-disks[i]-dst
+STREQ(def-disks[i]-dst, hdc)
+virDomainDiskGetSource(def-disks[i])) {
+if (xenXMConfigSetString(conf, cdrom,
+ virDomainDiskGetSource(def-disks[i])) 0)
+return -1;
+break;
+}
+}
+}
+}
-if (xenFormatXMMem(conf, def) 0)
-goto cleanup;
+return 0;
+}
-if (xenFormatXMCPUAllocation(conf, def) 0)
-goto cleanup;
-hvm = STREQ(def-os.type, hvm) ? 1 : 0;
+static int
+xenFormatXMOS(virConfPtr conf, virDomainDefPtr def)
+{
+size_t i;
-if (hvm) {
+if (STREQ(def-os.type, hvm)) {
char
Re: [libvirt] [PATCH] cmdMigrate: move vshConnect before vshWatchJob
On 8/8/2014 at 04:44 PM, in message
1407487476-28077-1-git-send-email-cy...@suse.com, Chunyan Liu
cy...@suse.com wrote:
A possible fix to issue:
http://www.redhat.com/archives/libvir-list/2014-August/thread.html#00227
While doing migration on KVM host, found problem sometimes:
VM is already running on the target host and disappears from source
host, but 'virsh migrate' command line hangs, cannot exit normally.
If pressing ENTER key, it will exit.
The code hangs at tools/virsh-domain.c: cmdMigrate
-vshWatchJob-poll():
poll() is trying to select pipe_fd, which is used to receive message
from doMigrate thread. In debugging, found that doMigrate finishes
and at the end it does call safewrite() to write the retval ('0' or
'1') to pipe_fd, and the write is completed. But cmdMigrate poll()
cannot get the event. If pressing ENTER key, poll() can get the
event and select pipe_fd, then command line can exit.
In current code, authentication thread which is called by vshConnect
will use stdin, and at the same time, in cmdMigrate main process,
poll() is listening to stdin, that probably affect poll() to get
pipe_fd event. Better to move authentication before vshWatchJob. With
this change, above problem does not exist.
Any comments about this patch?
Signed-off-by: Chunyan Liu cy...@suse.com
---
tools/virsh-domain.c | 26 --
tools/virsh.h| 1 +
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f7193cb..2562326 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8965,6 +8965,7 @@ doMigrate(void *opaque)
virTypedParameterPtr params = NULL;
int nparams = 0;
int maxparams = 0;
+virConnectPtr dconn = data-dconn;
sigemptyset(sigmask);
sigaddset(sigmask, SIGINT);
@@ -9079,18 +9080,12 @@ doMigrate(void *opaque)
ret = '0';
} else {
/* For traditional live migration, connect to the destination host
directly. */
-virConnectPtr dconn = NULL;
virDomainPtr ddom = NULL;
-dconn = vshConnect(ctl, desturi, false);
-if (!dconn)
-goto out;
-
if ((ddom = virDomainMigrate3(dom, dconn, params, nparams, flags)))
{
virDomainFree(ddom);
ret = '0';
}
-virConnectClose(dconn);
}
out:
@@ -9152,6 +9147,23 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
data.cmd = cmd;
data.writefd = p[1];
+if (vshCommandOptBool(cmd, p2p) || vshCommandOptBool(cmd, direct))
{
+data.dconn = NULL;
+} else {
+/* For traditional live migration, connect to the destination host.
*/
+virConnectPtr dconn = NULL;
+const char *desturi = NULL;
+
+if (vshCommandOptStringReq(ctl, cmd, desturi, desturi) 0)
+goto cleanup;
+
+dconn = vshConnect(ctl, desturi, false);
+if (!dconn)
+goto cleanup;
+
+data.dconn = dconn;
+}
+
if (virThreadCreate(workerThread,
true,
doMigrate,
@@ -9163,6 +9175,8 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
virThreadJoin(workerThread);
cleanup:
+if (data.dconn)
+virConnectClose(data.dconn);
virDomainFree(dom);
VIR_FORCE_CLOSE(p[0]);
VIR_FORCE_CLOSE(p[1]);
diff --git a/tools/virsh.h b/tools/virsh.h
index 7656407..b4df24b 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -371,6 +371,7 @@ struct _vshCtrlData {
vshControl *ctl;
const vshCmd *cmd;
int writefd;
+virConnectPtr dconn;
};
/* error handling */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainFSFreeze: report unless the agent supports mountpoints param
On 13.08.2014 17:42, Tomoki Sekiyama wrote:
This patch gives users a nicer error message when the QEMU guest agent is
not new enough to support 'guest-fsfreeze-freeze-list' command, which is
used by qemuDomainFSFreeze() to freeze specified filesystems only.
Before this patch, it was depending on the agent to complain about unknown
command:
# virsh domfsfreeze domain --mountpoint /mnt/point
error: Unable to freeze filesystems
error: internal error: unable to execute QEMU agent command 'guest-
fsfreeze-freeze-list': The command guest-fsfreeze-freeze-list has not been
found
After:
# virsh domfsfreeze domain --mountpoint /mnt/point
error: Unable to freeze filesystems
error: argument unsupported: this version of guest agent doesn't support
specifying mountpoints
Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@hds.com
---
src/qemu/qemu_agent.c | 79
-
1 file changed, 78 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index a10954a..8102b36 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1279,6 +1279,75 @@ void qemuAgentNotifyEvent(qemuAgentPtr mon,
}
}
+static int qemuAgentCommandSupported(qemuAgentPtr mon,
+ const char *cmdname)
+{
+int ret = -1;
+size_t i;
+int ndata;
+virJSONValuePtr cmd;
+virJSONValuePtr data;
+virJSONValuePtr reply = NULL;
+
+cmd = qemuAgentMakeCommand(guest-info, NULL);
+if (!cmd)
+return -1;
+
+if (qemuAgentCommand(mon, cmd, reply, false,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGet(reply, return))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-info reply was missing return data));
+goto cleanup;
+}
+
+if (!(data = virJSONValueObjectGet(data, supported_commands))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-info reply was missing supported_commands));
+goto cleanup;
+}
+
+if (data-type != VIR_JSON_TYPE_ARRAY) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-info supported_commands was not an array));
+goto cleanup;
+}
+
+ndata = virJSONValueArraySize(data);
+
+for (i = 0; i ndata; i++) {
+virJSONValuePtr entry = virJSONValueArrayGet(data, i);
+const char *name;
+
+if (!entry) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(array element missing in guest-info
+ supported_commands));
+goto cleanup;
+}
+
+if (!(name = virJSONValueObjectGetString(entry, name))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-info supported_commands was missing
name));
+goto cleanup;
+}
+
+if (strcmp(name, cmdname) == 0) {
+ret = 1;
+goto cleanup;
+}
+}
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
VIR_ENUM_DECL(qemuAgentShutdownMode);
VIR_ENUM_IMPL(qemuAgentShutdownMode,
@@ -1346,8 +1415,16 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char
**mountpoints,
return -1;
if (qemuAgentCommand(mon, cmd, reply, true,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0) {
+if (mountpoints nmountpoints) {
+if (qemuAgentCommandSupported(mon,
+ guest-fsfreeze-freeze-list) == 0)
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+ _(this version of guest agent doesn't
support
+ specifying mountpoints));
+}
goto cleanup;
+}
if (virJSONValueObjectGetNumberInt(reply, return, ret) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
It's not that I'm against this patch in general. I'm just against it now :) I
mean, with current implementation libvirt doesn't know what qemu-ga is it
talking to. The guest agent can come and go, without libvirt noticing anything
[1]. So in this specific case, guest-info can be executed by say newer qemu-ga
which does support guest-fsfreeze-freeze-list and thus direct libvirt to use
it. But then the freeze command is executed by downgraded qemu-ga so user will
still see the ugly error message.
At qemu monitor level it makes sense to query supported commands and
capabilities because monitor disappears if and only if the qemu process dies.
The assumptions is however not true with qemu-ga.
1: The long
Re: [libvirt] [PATCH] qemu: Issue rtc_reset_reinjection command after guest-set-time
On 13.08.2014 18:30, Eric Blake wrote:
On 08/13/2014 06:51 AM, Michal Privoznik wrote:
s/_/-/2 in the subject line
An advice appeared there on the qemu-devel list [1]. When a domain is
suspended and then resumed guest kernel is not aware of this. So we've
introduced virDomainSetTime API that resets the time within guest
using qemu-ga. On the other hand, qemu itself is trying to make RTC
beat faster to catch the difference. But if we don't tell qemu that
guest's time was reset via the other method, both mechanisms are
applied resulting in again wrong guest time. In order to avoid summing
both corrections we need to tell qemu that it should not use the RTC
injection if the guest time is set via guest agent.
1: http://www.mail-archive.com/qemu-devel@nongnu.org/msg236435.html
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
+++ b/src/qemu/qemu_driver.c
@@ -16879,6 +16879,16 @@ qemuDomainSetTime(virDomainPtr dom,
rv = qemuAgentSetTime(priv-agent, seconds, nseconds, rtcSync);
qemuDomainObjExitAgent(vm);
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(domain is not running));
+goto endjob;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv-mon);
+qemuDomainObjExitMonitor(driver, vm);
This forces the command to fail if qemu is too old to have
rtc-reset-reinjection but the agent is new enough to set time. Should
you make this code conditional on whether qemu supports the QMP command?
I'm not sure. If that's the case, both corrections will apply so guest
ends up with incorrect time anyway. And if the API is to guarantee
correctly set time, it must fail if such guarantees can't be made IMO.
}
+
+int
+qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand(rtc-reset-reinjection,
+ NULL)))
+return ret;
+
+ret = qemuMonitorJSONCommand(mon, cmd, reply);
+
+if (ret == 0)
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
Is it worth enhancing the testsuite to add coverage for this command and
expected response?
Yeah. I'll post v2.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: fix parsing 'cmd_per_lun' and 'max_sectors'
From: Mo yuxiang moyuxi...@huawei.com
commit d9504941 introduces two new attributes cmd_per_lun and
max_sectors same with the names QEMU uses for virtio-scsi.
But the case of parsing them is not exact. Change to parse
them if controller has driver element.
Signed-off-by: Mo yuxiang moyuxi...@huawei.com
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 934f6cb..5c762fa 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6295,10 +6295,11 @@ virDomainControllerDefParseXML(xmlNodePtr node,
cur = node-children;
while (cur != NULL) {
if (cur-type == XML_ELEMENT_NODE) {
-if (xmlStrEqual(cur-name, BAD_CAST driver))
+if (xmlStrEqual(cur-name, BAD_CAST driver)) {
queues = virXMLPropString(cur, queues);
cmd_per_lun = virXMLPropString(cur, cmd_per_lun);
max_sectors = virXMLPropString(cur, max_sectors);
+}
}
cur = cur-next;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 libvirt] qemu: Issue rtc-reset-reinjection command after guest-set-time
https://bugzilla.redhat.com/show_bug.cgi?id=1103245
An advice appeared there on the qemu-devel list [1]. When a domain is
suspended and then resumed guest kernel is not aware of this. So we've
introduced virDomainSetTime API that resets the time within guest
using qemu-ga. On the other hand, qemu itself is trying to make RTC
beat faster to catch the difference. But if we don't tell qemu that
guest's time was reset via the other method, both mechanisms are
applied resulting in again wrong guest time. In order to avoid summing
both corrections we need to tell qemu that it should not use the RTC
injection if the guest time is set via guest agent.
1: http://www.mail-archive.com/qemu-devel@nongnu.org/msg236435.html
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
Notes:
diff to v1:
-fixed command name in subject
-added testcase
src/qemu/qemu_driver.c | 10 ++
src/qemu/qemu_monitor.c | 33 +
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 21 +
src/qemu/qemu_monitor_json.h | 2 ++
tests/qemumonitorjsontest.c | 1 +
6 files changed, 69 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b6219ba..bdfd155 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16879,6 +16879,16 @@ qemuDomainSetTime(virDomainPtr dom,
rv = qemuAgentSetTime(priv-agent, seconds, nseconds, rtcSync);
qemuDomainObjExitAgent(vm);
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(domain is not running));
+goto endjob;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv-mon);
+qemuDomainObjExitMonitor(driver, vm);
+
if (rv 0)
goto endjob;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3d9f87b..77627bc 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4037,3 +4037,36 @@ qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
return qemuMonitorJSONGetGuestCPU(mon, arch, data);
}
+
+/**
+ * qemuMonitorRTCResetReinjection:
+ * @mon: Pointer to the monitor
+ *
+ * Issue rtc-reset-reinjection command.
+ * This should be used in cases where guest time is restored via
+ * guest agent so RTC injection is not needed (in fact it will
+ * confuse guest's RTC).
+ *
+ * Returns 0 on success
+ *-1 on error.
+ */
+int
+qemuMonitorRTCResetReinjection(qemuMonitorPtr mon)
+{
+
+VIR_DEBUG(mon=%p, mon);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(monitor must not be NULL));
+return -1;
+}
+
+if (!mon-json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(JSON monitor is required));
+return -1;
+}
+
+return qemuMonitorJSONRTCResetReinjection(mon);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index c3695f2..4fd6f01 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -790,6 +790,8 @@ int qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
virArch arch,
virCPUDataPtr *data);
+int qemuMonitorRTCResetReinjection(qemuMonitorPtr mon);
+
/**
* When running two dd process and using redirection, we need a
* shell that will not truncate files. These two strings serve that
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a62c02f..538110c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5851,3 +5851,24 @@ qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
return -1;
}
}
+
+int
+qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand(rtc-reset-reinjection,
+ NULL)))
+return ret;
+
+ret = qemuMonitorJSONCommand(mon, cmd, reply);
+
+if (ret == 0)
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 5f6c846..d8c9308 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -437,4 +437,6 @@ int qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon,
int qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
virArch arch,
virCPUDataPtr *data);
+
+int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon);
#endif /* QEMU_MONITOR_JSON_H */
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index baee80a..e3fb4f7 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2279,6 +2279,7 @@ mymain(void)
DO_TEST_SIMPLE(inject-nmi, qemuMonitorJSONInjectNMI);
Re: [libvirt] [PATCH v3 9/9] daemon: use socket activation with systemd
On Wed, Aug 13, 2014 at 04:10:12PM +0100, Daniel P. Berrange wrote: On Wed, Jul 23, 2014 at 04:27:13PM +0200, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- .gitignore | 1 + daemon/Makefile.am | 14 -- daemon/libvirtd.conf | 5 + daemon/libvirtd.service.in | 5 - daemon/libvirtd.socket.in | 6 ++ libvirt.spec.in| 26 +- 6 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 daemon/libvirtd.socket.in diff --git a/.gitignore b/.gitignore index 90fee91..9776ea1 100644 --- a/.gitignore +++ b/.gitignore @@ -60,6 +60,7 @@ /daemon/libvirtd.pod /daemon/libvirtd.policy /daemon/libvirtd.service +/daemon/libvirtd.socket /daemon/test_libvirtd.aug /docs/aclperms.htmlinc /docs/apibuild.py.stamp diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 00221ab..70b7655 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -55,6 +55,7 @@ EXTRA_DIST = \ libvirtd.policy.in \ libvirtd.sasl \ libvirtd.service.in \ + libvirtd.socket.in \ libvirtd.sysconf\ libvirtd.sysctl \ libvirtd.aug\ @@ -388,15 +389,18 @@ endif ! LIBVIRT_INIT_SCRIPT_UPSTART if LIBVIRT_INIT_SCRIPT_SYSTEMD SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system -BUILT_SOURCES += libvirtd.service +BUILT_SOURCES += libvirtd.service libvirtd.socket -install-init-systemd: install-sysconfig libvirtd.service +install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket $(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR) $(INSTALL_DATA) libvirtd.service \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service + $(INSTALL_DATA) libvirtd.socket \ + $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket uninstall-init-systemd: uninstall-sysconfig rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service + rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || : else ! LIBVIRT_INIT_SCRIPT_SYSTEMD install-init-systemd: @@ -420,6 +424,12 @@ libvirtd.service: libvirtd.service.in $(top_builddir)/config.status $ $@-t \ mv $@-t $@ +libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status + $(AM_V_GEN)sed \ + -e 's|[@]runstatedir[@]|$(runstatedir)|g' \ +$ $@-t \ + mv $@-t $@ + check-local: check-augeas diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index e5856d4..b644e81 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -77,6 +77,11 @@ # UNIX socket access controls # +# Beware that if you are changing *any* of these options, and you use +# socket activation with systemd, you need to adjust the settings in +# the libvirtd.socket file as well since it could impose a security +# risk if you rely on file permission checking only. + # Set the UNIX domain socket group ownership. This can be used to # allow a 'trusted' set of users access to management capabilities # without becoming root. diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 086da36..1759ac8 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -1,8 +1,3 @@ -# NB we don't use socket activation. When libvirtd starts it will -# spawn any virtual machines registered for autostart. We want this -# to occur on every boot, regardless of whether any client connects -# to a socket. Thus socket activation doesn't have any benefit - [Unit] Description=Virtualization daemon Before=libvirt-guests.service diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in new file mode 100644 index 000..86cc3f4 --- /dev/null +++ b/daemon/libvirtd.socket.in @@ -0,0 +1,6 @@ +[Socket] +ListenStream=@runstatedir@/libvirt/libvirt-sock +ListenStream=@runstatedir@/libvirt/libvirt-sock-ro +SocketMode=0777 +SocketUser=root +SocketGroup=root Perhaps add a comment in this file about Mode=0777 *only* being safe if you have libvirtd.conf doing authentication (eg polkit) on both UNIX sockets. [I'm starting to regret that I wanted to fix some simple timeout-before-error issue :)] I can add the comment, but I just realized that we can't ship it this way. If someone has no authentication set up and the socket allowed only for root (for example), the machine would be vulnerable after update to the version with this libvirtd.socket. If, on the other hand, we put here 0700 for example, lot of applications may stop working, because they rely on authentication with 0777. And the daemon can do chmod() on the socket
[libvirt] [PATCH] fix
Also add qemuDomainChangeGraphicsPasswords,
qemuProcessVerifyGuestCPU and qemuProcessInitPCIAddresses.
Replace tabs by spaces.
---
I'll do some testing on the patch and push it later with this squashed in
if it works well.
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_hotplug.c | 12
src/qemu/qemu_hotplug.h | 3 ++-
src/qemu/qemu_process.c | 33 -
4 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 59b2647..e9506e0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2507,7 +2507,7 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
- return -1;
+return -1;
if (qemuMonitorGetDeviceAliases(priv-mon, aliases) 0) {
qemuDomainObjExitMonitor(driver, vm);
return -1;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 004b6a4..f7e223a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2357,7 +2357,8 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
ret = qemuDomainChangeGraphicsPasswords(driver, vm,
VIR_DOMAIN_GRAPHICS_TYPE_VNC,
dev-data.vnc.auth,
-cfg-vncPassword);
+cfg-vncPassword,
+QEMU_ASYNC_JOB_NONE);
if (ret 0)
goto cleanup;
@@ -2407,7 +2408,8 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
ret = qemuDomainChangeGraphicsPasswords(driver, vm,
VIR_DOMAIN_GRAPHICS_TYPE_SPICE,
dev-data.spice.auth,
-cfg-spicePassword);
+cfg-spicePassword,
+QEMU_ASYNC_JOB_NONE);
if (ret 0)
goto cleanup;
@@ -3517,7 +3519,8 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int type,
virDomainGraphicsAuthDefPtr auth,
- const char *defaultPasswd)
+ const char *defaultPasswd,
+ int asyncJob)
{
qemuDomainObjPrivatePtr priv = vm-privateData;
time_t now = time(NULL);
@@ -3534,7 +3537,8 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
if (auth-connected)
connected =
virDomainGraphicsAuthConnectedTypeToString(auth-connected);
-qemuDomainObjEnterMonitor(driver, vm);
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
+goto cleanup;
ret = qemuMonitorSetPassword(priv-mon,
type,
auth-passwd ? auth-passwd : defaultPasswd,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 6192973..f626c0a 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -61,7 +61,8 @@ int qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int type,
virDomainGraphicsAuthDefPtr auth,
- const char *defaultPasswd);
+ const char *defaultPasswd,
+ int asyncJob);
int qemuDomainChangeNet(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainPtr dom,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 17f65b4..13c396f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1559,7 +1559,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver,
virDomainObjPtr vm, int asyncJob,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
- goto error;
+goto error;
ret = qemuMonitorSetCapabilities(priv-mon);
if (ret == 0
virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON))
@@ -2004,7 +2004,7 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver,
priv = vm-privateData;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
- goto cleanup;
+goto cleanup;
ret = qemuMonitorGetPtyPaths(priv-mon, paths);
qemuDomainObjExitMonitor(driver, vm);
@@ -2057,7 +2057,7 @@ qemuProcessDetectVcpuPIDs(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm-privateData;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
-
Re: [libvirt] [PATCH] conf: fix parsing 'cmd_per_lun' and 'max_sectors'
On 14.08.2014 09:55, Wang Rui wrote:
From: Mo yuxiang moyuxi...@huawei.com
commit d9504941 introduces two new attributes cmd_per_lun and
max_sectors same with the names QEMU uses for virtio-scsi.
But the case of parsing them is not exact. Change to parse
them if controller has driver element.
Signed-off-by: Mo yuxiang moyuxi...@huawei.com
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 934f6cb..5c762fa 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6295,10 +6295,11 @@ virDomainControllerDefParseXML(xmlNodePtr node,
cur = node-children;
while (cur != NULL) {
if (cur-type == XML_ELEMENT_NODE) {
-if (xmlStrEqual(cur-name, BAD_CAST driver))
+if (xmlStrEqual(cur-name, BAD_CAST driver)) {
queues = virXMLPropString(cur, queues);
cmd_per_lun = virXMLPropString(cur, cmd_per_lun);
max_sectors = virXMLPropString(cur, max_sectors);
+}
}
cur = cur-next;
}
ACKed and pushed. Unfortunately, the 1.2.7 release is broken so I'm
pushing this into 1.2.7 maint branch too.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: force configure failed when perl is missing
On Thu, Aug 14, 2014 at 11:37:45AM +0800, Jincheng Miao wrote: Perl is necessary to our build processing, it will invoke a lot of generating script, like: gendispatch.pl. If perl is missing, it's ok for build from git checkout, because autogen.sh will tell you. But for compiling from a release tarball, configure will just record a missing message, and continue, then build failed, like: https://www.redhat.com/archives/libvirt-users/2014-August/msg00050.html So need to enhance configure script to handle this negative case. Reported-by: Hongbin Lu hong...@savinetwork.ca Signed-off-by: Jincheng Miao jm...@redhat.com --- configure.ac |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 081f298..af3fe28 100644 --- a/configure.ac +++ b/configure.ac @@ -2173,6 +2173,9 @@ AM_CONDITIONAL([WITH_HYPERV], [test $with_hyperv = yes]) dnl Allow perl/python overrides AC_PATH_PROGS([PYTHON], [python2 python]) AC_PATH_PROG([PERL], [perl]) +if test -z $PERL; then + AC_MSG_ERROR([Failed to find perl.]) +fi AC_ARG_WITH([test-suite], [AS_HELP_STRING([--with-test-suite], ACK 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 v3 9/9] daemon: use socket activation with systemd
On Thu, Aug 14, 2014 at 10:38:42AM +0200, Martin Kletzander wrote: On Wed, Aug 13, 2014 at 04:10:12PM +0100, Daniel P. Berrange wrote: On Wed, Jul 23, 2014 at 04:27:13PM +0200, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- .gitignore | 1 + daemon/Makefile.am | 14 -- daemon/libvirtd.conf | 5 + daemon/libvirtd.service.in | 5 - daemon/libvirtd.socket.in | 6 ++ libvirt.spec.in| 26 +- 6 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 daemon/libvirtd.socket.in diff --git a/.gitignore b/.gitignore index 90fee91..9776ea1 100644 --- a/.gitignore +++ b/.gitignore @@ -60,6 +60,7 @@ /daemon/libvirtd.pod /daemon/libvirtd.policy /daemon/libvirtd.service +/daemon/libvirtd.socket /daemon/test_libvirtd.aug /docs/aclperms.htmlinc /docs/apibuild.py.stamp diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 00221ab..70b7655 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -55,6 +55,7 @@ EXTRA_DIST = \ libvirtd.policy.in \ libvirtd.sasl \ libvirtd.service.in \ + libvirtd.socket.in \ libvirtd.sysconf\ libvirtd.sysctl \ libvirtd.aug\ @@ -388,15 +389,18 @@ endif ! LIBVIRT_INIT_SCRIPT_UPSTART if LIBVIRT_INIT_SCRIPT_SYSTEMD SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system -BUILT_SOURCES += libvirtd.service +BUILT_SOURCES += libvirtd.service libvirtd.socket -install-init-systemd: install-sysconfig libvirtd.service +install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket $(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR) $(INSTALL_DATA) libvirtd.service \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service + $(INSTALL_DATA) libvirtd.socket \ + $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket uninstall-init-systemd: uninstall-sysconfig rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service + rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || : else ! LIBVIRT_INIT_SCRIPT_SYSTEMD install-init-systemd: @@ -420,6 +424,12 @@ libvirtd.service: libvirtd.service.in $(top_builddir)/config.status $ $@-t \ mv $@-t $@ +libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status + $(AM_V_GEN)sed \ + -e 's|[@]runstatedir[@]|$(runstatedir)|g' \ +$ $@-t \ + mv $@-t $@ + check-local: check-augeas diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index e5856d4..b644e81 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -77,6 +77,11 @@ # UNIX socket access controls # +# Beware that if you are changing *any* of these options, and you use +# socket activation with systemd, you need to adjust the settings in +# the libvirtd.socket file as well since it could impose a security +# risk if you rely on file permission checking only. + # Set the UNIX domain socket group ownership. This can be used to # allow a 'trusted' set of users access to management capabilities # without becoming root. diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 086da36..1759ac8 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -1,8 +1,3 @@ -# NB we don't use socket activation. When libvirtd starts it will -# spawn any virtual machines registered for autostart. We want this -# to occur on every boot, regardless of whether any client connects -# to a socket. Thus socket activation doesn't have any benefit - [Unit] Description=Virtualization daemon Before=libvirt-guests.service diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in new file mode 100644 index 000..86cc3f4 --- /dev/null +++ b/daemon/libvirtd.socket.in @@ -0,0 +1,6 @@ +[Socket] +ListenStream=@runstatedir@/libvirt/libvirt-sock +ListenStream=@runstatedir@/libvirt/libvirt-sock-ro +SocketMode=0777 +SocketUser=root +SocketGroup=root Perhaps add a comment in this file about Mode=0777 *only* being safe if you have libvirtd.conf doing authentication (eg polkit) on both UNIX sockets. [I'm starting to regret that I wanted to fix some simple timeout-before-error issue :)] I can add the comment, but I just realized that we can't ship it this way. If someone has no authentication set up and the socket allowed only for root (for example), the machine would be vulnerable after update to the version with this libvirtd.socket. If, on the other hand, we put here 0700 for example, lot of applications
Re: [libvirt] [PATCH v3 9/9] daemon: use socket activation with systemd
On Thu, Aug 14, 2014 at 10:07:34AM +0100, Daniel P. Berrange wrote: On Thu, Aug 14, 2014 at 10:38:42AM +0200, Martin Kletzander wrote: On Wed, Aug 13, 2014 at 04:10:12PM +0100, Daniel P. Berrange wrote: On Wed, Jul 23, 2014 at 04:27:13PM +0200, Martin Kletzander wrote: diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in new file mode 100644 index 000..86cc3f4 --- /dev/null +++ b/daemon/libvirtd.socket.in @@ -0,0 +1,6 @@ +[Socket] +ListenStream=@runstatedir@/libvirt/libvirt-sock +ListenStream=@runstatedir@/libvirt/libvirt-sock-ro +SocketMode=0777 +SocketUser=root +SocketGroup=root Perhaps add a comment in this file about Mode=0777 *only* being safe if you have libvirtd.conf doing authentication (eg polkit) on both UNIX sockets. [I'm starting to regret that I wanted to fix some simple timeout-before-error issue :)] I can add the comment, but I just realized that we can't ship it this way. If someone has no authentication set up and the socket allowed only for root (for example), the machine would be vulnerable after update to the version with this libvirtd.socket. If, on the other hand, we put here 0700 for example, lot of applications may stop working, because they rely on authentication with 0777. And the daemon can do chmod() on the socket *only* to more lax permissions (not the other way around, as it would result in the same problem why we needed to add the comment in the first place). The solutions I came up with are: - Have SocketMode=0700 and do chmod() in the daemon to adjust the mode to permissions from the config file. And the better one: - Drop this whole socket starting stuff, because if there's a race, it's a systemd's problem. We call sd_notify(0, READY=1) when everything is set up as systemd wants us to! I just discovered that now. Actually there's a 3rd option - Don't run 'systemctl enable libvirtd.socket' That way we provide the ability to use it, but don't turn it on - people have to explicitly opt-in. I still don't see what would be the added value, but it certainly is. I'll do that (with the added comment) in next version. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 8/9] rpc: pass listen FD to the daemon being started
On Wed, Aug 13, 2014 at 04:09:09PM +0100, Daniel P. Berrange wrote:
On Wed, Jul 23, 2014 at 04:27:12PM +0200, Martin Kletzander wrote:
This eliminates the need for active waiting.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/virnetsocket.c | 102 -
1 file changed, 83 insertions(+), 19 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index a94b2bc..46be541 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1,7 +1,7 @@
/*
* virnetsocket.c: generic network socket handling
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -121,7 +121,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket)
#ifndef WIN32
-static int virNetSocketForkDaemon(const char *binary)
+static int virNetSocketForkDaemon(const char *binary, int passfd)
s/int/bool/ perhaps ?
Which one you mean? I'm keeping the return value as it was and the
passfd is the fd that will be passed.
[...]
@@ -569,28 +574,82 @@ int virNetSocketNewConnectUNIX(const char *path,
if (remoteAddr.data.un.sun_path[0] == '@')
remoteAddr.data.un.sun_path[0] = '\0';
- retry:
-if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
-if ((errno == ECONNREFUSED ||
- errno == ENOENT)
-spawnDaemon retries 20) {
-VIR_DEBUG(Connection refused for %s, trying to spawn %s,
- path, binary);
-if (retries == 0
-virNetSocketForkDaemon(binary) 0)
-goto error;
+if (spawnDaemon) {
+int err = 0;
+int rv = -1;
+int status = 0;
+pid_t pid = 0;
-retries++;
-usleep(1000 * 100 * retries);
-goto retry;
+if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) {
+virReportSystemError(errno, %s, _(Failed to create socket));
+goto error;
}
-virReportSystemError(errno,
- _(Failed to connect socket to '%s'),
+if (pipe2(errfd, O_CLOEXEC) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot create pipe for child));
+goto error;
+}
+
+/*
+ * We have to fork() here, because umask() is set
+ * per-process, chmod() is racy and fchmod() has undefined
+ * behaviour on sockets according to POSIX, so it doesn't
+ * work outside Linux.
+ */
+
+if ((pid = virFork()) 0)
+goto error;
+
+if (pid == 0) {
+VIR_FORCE_CLOSE(errfd[0]);
+
+umask(0077);
+rv = bind(passfd, remoteAddr.data.sa, remoteAddr.len);
+if (rv 0) {
+ignore_value(safewrite(errfd[1], errno, sizeof(int)));
+}
+VIR_FORCE_CLOSE(errfd[1]);
+_exit(rv 0 ? EXIT_FAILURE : EXIT_SUCCESS);
+}
+
+VIR_FORCE_CLOSE(errfd[1]);
+rv = virProcessWait(pid, status, false);
+ignore_value(saferead(errfd[0], err, sizeof(int)));
+VIR_FORCE_CLOSE(errfd[0]);
+
+if (rv 0 || status != EXIT_SUCCESS) {
+if (err) {
+virReportSystemError(err,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+}
+goto error;
+}
+
+if (listen(passfd, 0) 0) {
+virReportSystemError(errno, %s,
+ _(Failed to listen on socket that's about
+ to be passed to the daemon));
+goto error;
+}
Perhaps I'm miss-reading the code, but isn't this block gonig to
result in failure if libvirtd is already running ( listening on
the socket we try to bind to) and we requested auto-spawn ?
+}
+
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
+virReportSystemError(errno, _(Failed to connect socket to '%s'),
path);
goto error;
}
Also I think there's still a race here because the already running
libvirtd daemon could be unfortunately shutting down right at the
same time we try to connect. So I think we still need to have a
re-try loop around the connect attempt to address that race. Obviously
it should be pretty damn rare now so won't have the problems that the
current loop has.
Wouldn't connect(), bind(), connect() be enough (for both issues)? Or
Re: [libvirt] [PATCH v3 8/9] rpc: pass listen FD to the daemon being started
On Thu, Aug 14, 2014 at 11:23:27AM +0200, Martin Kletzander wrote:
On Wed, Aug 13, 2014 at 04:09:09PM +0100, Daniel P. Berrange wrote:
On Wed, Jul 23, 2014 at 04:27:12PM +0200, Martin Kletzander wrote:
This eliminates the need for active waiting.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/virnetsocket.c | 102
-
1 file changed, 83 insertions(+), 19 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index a94b2bc..46be541 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1,7 +1,7 @@
/*
* virnetsocket.c: generic network socket handling
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -121,7 +121,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket)
#ifndef WIN32
-static int virNetSocketForkDaemon(const char *binary)
+static int virNetSocketForkDaemon(const char *binary, int passfd)
s/int/bool/ perhaps ?
Which one you mean? I'm keeping the return value as it was and the
passfd is the fd that will be passed.
Never mind, I mis-interpreted 'passfd' as an instruction not a
value.
[...]
@@ -569,28 +574,82 @@ int virNetSocketNewConnectUNIX(const char *path,
if (remoteAddr.data.un.sun_path[0] == '@')
remoteAddr.data.un.sun_path[0] = '\0';
- retry:
-if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
-if ((errno == ECONNREFUSED ||
- errno == ENOENT)
-spawnDaemon retries 20) {
-VIR_DEBUG(Connection refused for %s, trying to spawn %s,
- path, binary);
-if (retries == 0
-virNetSocketForkDaemon(binary) 0)
-goto error;
+if (spawnDaemon) {
+int err = 0;
+int rv = -1;
+int status = 0;
+pid_t pid = 0;
-retries++;
-usleep(1000 * 100 * retries);
-goto retry;
+if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) {
+virReportSystemError(errno, %s, _(Failed to create
socket));
+goto error;
}
-virReportSystemError(errno,
- _(Failed to connect socket to '%s'),
+if (pipe2(errfd, O_CLOEXEC) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot create pipe for child));
+goto error;
+}
+
+/*
+ * We have to fork() here, because umask() is set
+ * per-process, chmod() is racy and fchmod() has undefined
+ * behaviour on sockets according to POSIX, so it doesn't
+ * work outside Linux.
+ */
+
+if ((pid = virFork()) 0)
+goto error;
+
+if (pid == 0) {
+VIR_FORCE_CLOSE(errfd[0]);
+
+umask(0077);
+rv = bind(passfd, remoteAddr.data.sa, remoteAddr.len);
+if (rv 0) {
+ignore_value(safewrite(errfd[1], errno, sizeof(int)));
+}
+VIR_FORCE_CLOSE(errfd[1]);
+_exit(rv 0 ? EXIT_FAILURE : EXIT_SUCCESS);
+}
+
+VIR_FORCE_CLOSE(errfd[1]);
+rv = virProcessWait(pid, status, false);
+ignore_value(saferead(errfd[0], err, sizeof(int)));
+VIR_FORCE_CLOSE(errfd[0]);
+
+if (rv 0 || status != EXIT_SUCCESS) {
+if (err) {
+virReportSystemError(err,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+}
+goto error;
+}
+
+if (listen(passfd, 0) 0) {
+virReportSystemError(errno, %s,
+ _(Failed to listen on socket that's
about
+ to be passed to the daemon));
+goto error;
+}
Perhaps I'm miss-reading the code, but isn't this block gonig to
result in failure if libvirtd is already running ( listening on
the socket we try to bind to) and we requested auto-spawn ?
+}
+
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
+virReportSystemError(errno, _(Failed to connect socket to '%s'),
path);
goto error;
}
Also I think there's still a race here because the already running
libvirtd daemon could be unfortunately shutting down right at the
same time we try to
Re: [libvirt] [PATCH v3 4/9] daemon: support passing FDs from the calling process
On Wed, Aug 13, 2014 at 04:01:42PM +0100, Daniel P. Berrange wrote:
On Wed, Jul 23, 2014 at 04:27:08PM +0200, Martin Kletzander wrote:
First FD is the RW unix socket to listen on, second one (if
applicable) is the RO unix socket.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
daemon/libvirtd.c | 45 +++--
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 4c926b3..15f0ce2 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -56,6 +56,7 @@
#include virstring.h
#include locking/lock_manager.h
#include viraccessmanager.h
+#include virutil.h
#ifdef WITH_DRIVER_MODULES
# include driver.h
@@ -476,11 +477,19 @@ static int daemonSetupNetworking(virNetServerPtr srv,
int unix_sock_ro_mask = 0;
int unix_sock_rw_mask = 0;
+unsigned int cur_fd = STDERR_FILENO + 1;
+unsigned int nfds = virGetListenFDs();
+
if (config-unix_sock_group) {
if (virGetGroupID(config-unix_sock_group, unix_sock_gid) 0)
return -1;
}
+if (nfds nfds ((int)!!sock_path + (int)!!sock_path_ro)) {
+VIR_ERROR(_(Too many (%u) FDs passed from caller), nfds);
+return -1;
+}
+
if (virStrToLong_i(config-unix_sock_ro_perms, NULL, 8,
unix_sock_ro_mask) != 0) {
VIR_ERROR(_(Failed to parse mode '%s'), config-unix_sock_ro_perms);
goto error;
@@ -491,30 +500,30 @@ static int daemonSetupNetworking(virNetServerPtr srv,
goto error;
}
-VIR_DEBUG(Registering unix socket %s, sock_path);
-if (!(svc = virNetServerServiceNewUNIX(sock_path,
- unix_sock_rw_mask,
- unix_sock_gid,
- config-auth_unix_rw,
+if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path,
+ unix_sock_rw_mask,
+ unix_sock_gid,
+ config-auth_unix_rw,
#if WITH_GNUTLS
- NULL,
+ NULL,
#endif
- false,
- config-max_queued_clients,
- config-max_client_requests)))
+ false,
+ config-max_queued_clients,
+ config-max_client_requests,
+ nfds, cur_fd)))
goto error;
if (sock_path_ro) {
-VIR_DEBUG(Registering unix socket %s, sock_path_ro);
-if (!(svcRO = virNetServerServiceNewUNIX(sock_path_ro,
- unix_sock_ro_mask,
- unix_sock_gid,
- config-auth_unix_ro,
+if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro,
+ unix_sock_ro_mask,
+ unix_sock_gid,
+ config-auth_unix_ro,
#if WITH_GNUTLS
- NULL,
+ NULL,
#endif
- true,
- config-max_queued_clients,
- config-max_client_requests)))
+ true,
+
config-max_queued_clients,
+
config-max_client_requests,
+ nfds, cur_fd)))
goto error;
}
I'm wondering how we deal with TCP socket listening too. eg if someone
modifies the systemd config so that it listens on the TCP sockets too
then we're not setting up libvirt to deal with those.
Is this really necessary? I mean I went with the approach that we can
get more than 1 FD passed even though it's not needed to fix the 20s
timeout (the original bug). And if we decide we need more it can be
reworked that way, yes. But right now I really don't think we do,
especially since race condition solved by passing TCP or TLS ports
would be *really* rare; and anyone connecting during the time system
daemon is starting will just try again in a while.
Rather than doing this NewFDOrUnix() approach, I'm wondering if we
would be better off doing.
if (getenv(LISTEN_FDS) != NULL) {
call NewFD() on all the FDs we've been given...
} else {
.. do normal NewUnix/NewTCP/NewTLS setup...
}
though it gets harder than that because we'll need
Re: [libvirt] [PATCH] build: force configure failed when perl is missing
On 14.08.2014 05:37, Jincheng Miao wrote:
Perl is necessary to our build processing, it will invoke a lot of
generating script, like: gendispatch.pl. If perl is missing, it's
ok for build from git checkout, because autogen.sh will tell you.
But for compiling from a release tarball, configure will just record
a missing message, and continue, then build failed, like:
https://www.redhat.com/archives/libvirt-users/2014-August/msg00050.html
So need to enhance configure script to handle this negative case.
Reported-by: Hongbin Lu hong...@savinetwork.ca
Signed-off-by: Jincheng Miao jm...@redhat.com
---
configure.ac |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac
index 081f298..af3fe28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2173,6 +2173,9 @@ AM_CONDITIONAL([WITH_HYPERV], [test $with_hyperv =
yes])
dnl Allow perl/python overrides
AC_PATH_PROGS([PYTHON], [python2 python])
AC_PATH_PROG([PERL], [perl])
+if test -z $PERL; then
+ AC_MSG_ERROR([Failed to find perl.])
+fi
AC_ARG_WITH([test-suite],
[AS_HELP_STRING([--with-test-suite],
I'm inclined to ACK this. We currently have some files that can be built
from git and are contained in the release so we cut off the set of
required tools to build the libvirt. However, with so widely accessible
tool as perl (which is almost everywhere) we don't need to do that.
Moreover, there are some Makefile targets which have runtime dependency
on perl, e.g. bracket-spacing-check syntax-check rule.
And with this change I think we need this one too:
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 29da071..f491de7 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -446,6 +446,7 @@ BuildRequires: gettext-devel
BuildRequires: libtool
BuildRequires: /usr/bin/pod2man
%endif
+BuildRequires: perl
BuildRequires: python
%if %{with_systemd}
BuildRequires: systemd-units
Again, I'm not expecting to see any RPM based distribution without perl
(esp. if rpm-build package requires perl itself). But we should have it
for completeness.
So I'm squashing it in and pushing.
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix
On 14.08.2014 10:41, Ján Tomko wrote: Also add qemuDomainChangeGraphicsPasswords, qemuProcessVerifyGuestCPU and qemuProcessInitPCIAddresses. Replace tabs by spaces. --- I'll do some testing on the patch and push it later with this squashed in if it works well. src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_hotplug.c | 12 src/qemu/qemu_hotplug.h | 3 ++- src/qemu/qemu_process.c | 33 - 4 files changed, 31 insertions(+), 19 deletions(-) ACK to the original with this squashed in. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix
On 08/14/2014 11:52 AM, Michal Privoznik wrote: On 14.08.2014 10:41, Ján Tomko wrote: Also add qemuDomainChangeGraphicsPasswords, qemuProcessVerifyGuestCPU and qemuProcessInitPCIAddresses. Replace tabs by spaces. --- I'll do some testing on the patch and push it later with this squashed in if it works well. src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_hotplug.c | 12 src/qemu/qemu_hotplug.h | 3 ++- src/qemu/qemu_process.c | 33 - 4 files changed, 31 insertions(+), 19 deletions(-) ACK to the original with this squashed in. Now pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: fix missing forward slash
Should like below: interface type='server' mac address='52:54:00:22:c9:42'/ source address='192.168.0.1' port='5558'/ /interface ... interface type='client' mac address='52:54:00:8b:c9:51'/ source address='192.168.0.1' port='5558'/ /interface --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bd99ae0..ed17389 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3701,12 +3701,12 @@ ... lt;devicesgt; lt;interface type='server'gt; - lt;mac address='52:54:00:22:c9:42'gt; + lt;mac address='52:54:00:22:c9:42'/gt; lt;source address='192.168.0.1' port='5558'/gt; lt;/interfacegt; ... lt;interface type='client'gt; - lt;mac address='52:54:00:8b:c9:51'gt; + lt;mac address='52:54:00:8b:c9:51'/gt; lt;source address='192.168.0.1' port='5558'/gt; lt;/interfacegt; lt;/devicesgt; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 8/9] rpc: pass listen FD to the daemon being started
On Thu, Aug 14, 2014 at 10:26:41AM +0100, Daniel P. Berrange wrote:
On Thu, Aug 14, 2014 at 11:23:27AM +0200, Martin Kletzander wrote:
On Wed, Aug 13, 2014 at 04:09:09PM +0100, Daniel P. Berrange wrote:
On Wed, Jul 23, 2014 at 04:27:12PM +0200, Martin Kletzander wrote:
This eliminates the need for active waiting.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/virnetsocket.c | 102
-
1 file changed, 83 insertions(+), 19 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
[...]
@@ -569,28 +574,82 @@ int virNetSocketNewConnectUNIX(const char *path,
if (remoteAddr.data.un.sun_path[0] == '@')
remoteAddr.data.un.sun_path[0] = '\0';
- retry:
-if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
-if ((errno == ECONNREFUSED ||
- errno == ENOENT)
-spawnDaemon retries 20) {
-VIR_DEBUG(Connection refused for %s, trying to spawn %s,
- path, binary);
-if (retries == 0
-virNetSocketForkDaemon(binary) 0)
-goto error;
+if (spawnDaemon) {
+int err = 0;
+int rv = -1;
+int status = 0;
+pid_t pid = 0;
-retries++;
-usleep(1000 * 100 * retries);
-goto retry;
+if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) {
+virReportSystemError(errno, %s, _(Failed to create socket));
+goto error;
}
-virReportSystemError(errno,
- _(Failed to connect socket to '%s'),
+if (pipe2(errfd, O_CLOEXEC) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot create pipe for child));
+goto error;
+}
+
+/*
+ * We have to fork() here, because umask() is set
+ * per-process, chmod() is racy and fchmod() has undefined
+ * behaviour on sockets according to POSIX, so it doesn't
+ * work outside Linux.
+ */
+
+if ((pid = virFork()) 0)
+goto error;
+
+if (pid == 0) {
+VIR_FORCE_CLOSE(errfd[0]);
+
+umask(0077);
+rv = bind(passfd, remoteAddr.data.sa, remoteAddr.len);
+if (rv 0) {
+ignore_value(safewrite(errfd[1], errno, sizeof(int)));
+}
+VIR_FORCE_CLOSE(errfd[1]);
+_exit(rv 0 ? EXIT_FAILURE : EXIT_SUCCESS);
+}
+
+VIR_FORCE_CLOSE(errfd[1]);
+rv = virProcessWait(pid, status, false);
+ignore_value(saferead(errfd[0], err, sizeof(int)));
+VIR_FORCE_CLOSE(errfd[0]);
+
+if (rv 0 || status != EXIT_SUCCESS) {
+if (err) {
+virReportSystemError(err,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+}
+goto error;
+}
+
+if (listen(passfd, 0) 0) {
+virReportSystemError(errno, %s,
+ _(Failed to listen on socket that's about
+ to be passed to the daemon));
+goto error;
+}
Perhaps I'm miss-reading the code, but isn't this block gonig to
result in failure if libvirtd is already running ( listening on
the socket we try to bind to) and we requested auto-spawn ?
+}
+
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
+virReportSystemError(errno, _(Failed to connect socket to '%s'),
path);
goto error;
}
Also I think there's still a race here because the already running
libvirtd daemon could be unfortunately shutting down right at the
same time we try to connect. So I think we still need to have a
re-try loop around the connect attempt to address that race. Obviously
it should be pretty damn rare now so won't have the problems that the
current loop has.
Wouldn't connect(), bind(), connect() be enough (for both issues)? Or
do we need to try the dance few times again?
We need to retry the whole block including daemon spawn to be able to
handle it if the existing daemon is in the process of shutting down
I believe.
I probably expressed myself badly with that connect, bind, connect.
This was the difference I was talking about:
diff --git i/src/rpc/virnetsocket.c w/src/rpc/virnetsocket.c
index 46be541..a4d5dd5 100644
--- i/src/rpc/virnetsocket.c
+++ w/src/rpc/virnetsocket.c
@@ -574,7 +574,12 @@ int virNetSocketNewConnectUNIX(const char *path,
if
Re: [libvirt] [PATCH v3 8/9] rpc: pass listen FD to the daemon being started
On Thu, Aug 14, 2014 at 01:02:28PM +0200, Martin Kletzander wrote:
On Thu, Aug 14, 2014 at 10:26:41AM +0100, Daniel P. Berrange wrote:
On Thu, Aug 14, 2014 at 11:23:27AM +0200, Martin Kletzander wrote:
On Wed, Aug 13, 2014 at 04:09:09PM +0100, Daniel P. Berrange wrote:
On Wed, Jul 23, 2014 at 04:27:12PM +0200, Martin Kletzander wrote:
This eliminates the need for active waiting.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/virnetsocket.c | 102
-
1 file changed, 83 insertions(+), 19 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
[...]
@@ -569,28 +574,82 @@ int virNetSocketNewConnectUNIX(const char *path,
if (remoteAddr.data.un.sun_path[0] == '@')
remoteAddr.data.un.sun_path[0] = '\0';
- retry:
-if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
-if ((errno == ECONNREFUSED ||
- errno == ENOENT)
-spawnDaemon retries 20) {
-VIR_DEBUG(Connection refused for %s, trying to spawn %s,
- path, binary);
-if (retries == 0
-virNetSocketForkDaemon(binary) 0)
-goto error;
+if (spawnDaemon) {
+int err = 0;
+int rv = -1;
+int status = 0;
+pid_t pid = 0;
-retries++;
-usleep(1000 * 100 * retries);
-goto retry;
+if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) {
+virReportSystemError(errno, %s, _(Failed to create
socket));
+goto error;
}
-virReportSystemError(errno,
- _(Failed to connect socket to '%s'),
+if (pipe2(errfd, O_CLOEXEC) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot create pipe for child));
+goto error;
+}
+
+/*
+ * We have to fork() here, because umask() is set
+ * per-process, chmod() is racy and fchmod() has undefined
+ * behaviour on sockets according to POSIX, so it doesn't
+ * work outside Linux.
+ */
+
+if ((pid = virFork()) 0)
+goto error;
+
+if (pid == 0) {
+VIR_FORCE_CLOSE(errfd[0]);
+
+umask(0077);
+rv = bind(passfd, remoteAddr.data.sa, remoteAddr.len);
+if (rv 0) {
+ignore_value(safewrite(errfd[1], errno, sizeof(int)));
+}
+VIR_FORCE_CLOSE(errfd[1]);
+_exit(rv 0 ? EXIT_FAILURE : EXIT_SUCCESS);
+}
+
+VIR_FORCE_CLOSE(errfd[1]);
+rv = virProcessWait(pid, status, false);
+ignore_value(saferead(errfd[0], err, sizeof(int)));
+VIR_FORCE_CLOSE(errfd[0]);
+
+if (rv 0 || status != EXIT_SUCCESS) {
+if (err) {
+virReportSystemError(err,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to bind socket to %s
+ in child process),
+ path);
+}
+goto error;
+}
+
+if (listen(passfd, 0) 0) {
+virReportSystemError(errno, %s,
+ _(Failed to listen on socket that's
about
+ to be passed to the daemon));
+goto error;
+}
Perhaps I'm miss-reading the code, but isn't this block gonig to
result in failure if libvirtd is already running ( listening on
the socket we try to bind to) and we requested auto-spawn ?
+}
+
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
+virReportSystemError(errno, _(Failed to connect socket to
'%s'),
path);
goto error;
}
Also I think there's still a race here because the already running
libvirtd daemon could be unfortunately shutting down right at the
same time we try to connect. So I think we still need to have a
re-try loop around the connect attempt to address that race. Obviously
it should be pretty damn rare now so won't have the problems that the
current loop has.
Wouldn't connect(), bind(), connect() be enough (for both issues)? Or
do we need to try the dance few times again?
We need to retry the whole block including daemon spawn to be able to
handle it if the existing daemon is in the process of shutting down
I believe.
I probably expressed myself badly with that connect, bind, connect.
This was the difference I was talking about:
diff --git i/src/rpc/virnetsocket.c
[libvirt] [PATCH] qemu: Redundant listen address entry in quest xml
When editing guest's XML (on QEMU), it was possible to add multiple
listen elements into graphics parent element. However QEMU does not
support listening on multiple addresses. This patch causes qemu
post-parse callback to remove any redundant entries, leaving only 1
listening address if provided, otherwise the configuration remains
untouched.
---
src/qemu/qemu_domain.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4f63c88..75a4446 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -827,6 +827,25 @@ qemuDomainDefPostParse(virDomainDefPtr def,
VIR_DOMAIN_INPUT_BUS_USB) 0)
return -1;
+/* loop over all graphics connections and all listening addresses,
+ * removing all redundant listening address entries, thus leaving
+ * only 1 entry
+ */
+if (def-ngraphics 0 def-graphics) {
+size_t i, j;
+for (i = 0; i def-ngraphics; i++) {
+virDomainGraphicsListenDefPtr listens = def-graphics[i]-listens;
+size_t nListens = def-graphics[i]-nListens;
+if (nListens = 1 || !listens)
+continue;
+for (j = 1; j nListens; j++) {
+VIR_FREE(listens[j].address);
+VIR_FREE(listens[j].network);
+}
+def-graphics[i]-nListens = 1;
+}
+}
+
return 0;
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Redundant listen address entry in quest xml
On Thu, Aug 14, 2014 at 01:09:32PM +0200, Erik Skultety wrote:
When editing guest's XML (on QEMU), it was possible to add multiple
listen elements into graphics parent element. However QEMU does not
support listening on multiple addresses. This patch causes qemu
post-parse callback to remove any redundant entries, leaving only 1
listening address if provided, otherwise the configuration remains
untouched.
Discarding part of the users requested config like this is not
the right thing todo. If the user requests multiple listen addresses
and we cannot honour that request that we must report that as an
error with code VIR_ERR_CONFIG_UNSUPPORTED
---
src/qemu/qemu_domain.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4f63c88..75a4446 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -827,6 +827,25 @@ qemuDomainDefPostParse(virDomainDefPtr def,
VIR_DOMAIN_INPUT_BUS_USB) 0)
return -1;
+/* loop over all graphics connections and all listening addresses,
+ * removing all redundant listening address entries, thus leaving
+ * only 1 entry
+ */
+if (def-ngraphics 0 def-graphics) {
+size_t i, j;
+for (i = 0; i def-ngraphics; i++) {
+virDomainGraphicsListenDefPtr listens =
def-graphics[i]-listens;
+size_t nListens = def-graphics[i]-nListens;
+if (nListens = 1 || !listens)
+continue;
+for (j = 1; j nListens; j++) {
+VIR_FREE(listens[j].address);
+VIR_FREE(listens[j].network);
+}
+def-graphics[i]-nListens = 1;
+}
+}
+
return 0;
}
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
[libvirt] [PATCH v4 4/9] daemon: support passing FDs from the calling process
First FD is the RW unix socket to listen on, second one (if
applicable) is the RO unix socket.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
daemon/libvirtd.c | 45 +++--
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index a1f64ad..2c18c5e 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -56,6 +56,7 @@
#include virstring.h
#include locking/lock_manager.h
#include viraccessmanager.h
+#include virutil.h
#ifdef WITH_DRIVER_MODULES
# include driver.h
@@ -476,11 +477,19 @@ static int daemonSetupNetworking(virNetServerPtr srv,
int unix_sock_ro_mask = 0;
int unix_sock_rw_mask = 0;
+unsigned int cur_fd = STDERR_FILENO + 1;
+unsigned int nfds = virGetListenFDs();
+
if (config-unix_sock_group) {
if (virGetGroupID(config-unix_sock_group, unix_sock_gid) 0)
return -1;
}
+if (nfds nfds ((int)!!sock_path + (int)!!sock_path_ro)) {
+VIR_ERROR(_(Too many (%u) FDs passed from caller), nfds);
+return -1;
+}
+
if (virStrToLong_i(config-unix_sock_ro_perms, NULL, 8,
unix_sock_ro_mask) != 0) {
VIR_ERROR(_(Failed to parse mode '%s'), config-unix_sock_ro_perms);
goto error;
@@ -491,30 +500,30 @@ static int daemonSetupNetworking(virNetServerPtr srv,
goto error;
}
-VIR_DEBUG(Registering unix socket %s, sock_path);
-if (!(svc = virNetServerServiceNewUNIX(sock_path,
- unix_sock_rw_mask,
- unix_sock_gid,
- config-auth_unix_rw,
+if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path,
+ unix_sock_rw_mask,
+ unix_sock_gid,
+ config-auth_unix_rw,
#if WITH_GNUTLS
- NULL,
+ NULL,
#endif
- false,
- config-max_queued_clients,
- config-max_client_requests)))
+ false,
+ config-max_queued_clients,
+ config-max_client_requests,
+ nfds, cur_fd)))
goto error;
if (sock_path_ro) {
-VIR_DEBUG(Registering unix socket %s, sock_path_ro);
-if (!(svcRO = virNetServerServiceNewUNIX(sock_path_ro,
- unix_sock_ro_mask,
- unix_sock_gid,
- config-auth_unix_ro,
+if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro,
+ unix_sock_ro_mask,
+ unix_sock_gid,
+ config-auth_unix_ro,
#if WITH_GNUTLS
- NULL,
+ NULL,
#endif
- true,
- config-max_queued_clients,
- config-max_client_requests)))
+ true,
+
config-max_queued_clients,
+
config-max_client_requests,
+ nfds, cur_fd)))
goto error;
}
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 8/9] rpc: pass listen FD to the daemon being started
This eliminates the need for active waiting.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/virnetsocket.c | 85 +++---
1 file changed, 66 insertions(+), 19 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index a94b2bc..f913365 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1,7 +1,7 @@
/*
* virnetsocket.c: generic network socket handling
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -121,7 +121,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket)
#ifndef WIN32
-static int virNetSocketForkDaemon(const char *binary)
+static int virNetSocketForkDaemon(const char *binary, int passfd)
{
int ret;
virCommandPtr cmd = virCommandNewArgList(binary,
@@ -134,6 +134,10 @@ static int virNetSocketForkDaemon(const char *binary)
virCommandAddEnvPassBlockSUID(cmd, XDG_RUNTIME_DIR, NULL);
virCommandClearCaps(cmd);
virCommandDaemonize(cmd);
+if (passfd) {
+virCommandPassFD(cmd, passfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+virCommandPassListenFDs(cmd);
+}
ret = virCommandRun(cmd, NULL);
virCommandFree(cmd);
return ret;
@@ -540,10 +544,10 @@ int virNetSocketNewConnectUNIX(const char *path,
const char *binary,
virNetSocketPtr *retsock)
{
+char *buf = NULL;
+int fd, passfd;
virSocketAddr localAddr;
virSocketAddr remoteAddr;
-int fd;
-int retries = 0;
memset(localAddr, 0, sizeof(localAddr));
memset(remoteAddr, 0, sizeof(remoteAddr));
@@ -570,25 +574,65 @@ int virNetSocketNewConnectUNIX(const char *path,
remoteAddr.data.un.sun_path[0] = '\0';
retry:
-if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
-if ((errno == ECONNREFUSED ||
- errno == ENOENT)
-spawnDaemon retries 20) {
-VIR_DEBUG(Connection refused for %s, trying to spawn %s,
- path, binary);
-if (retries == 0
-virNetSocketForkDaemon(binary) 0)
-goto error;
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0 !spawnDaemon) {
+virReportSystemError(errno, _(Failed to connect socket to '%s'),
+ path);
+goto error;
+} else if (spawnDaemon) {
+int status = 0;
+pid_t pid = 0;
+
+if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) {
+virReportSystemError(errno, %s, _(Failed to create socket));
+goto error;
+}
+
+/*
+ * We have to fork() here, because umask() is set
+ * per-process, chmod() is racy and fchmod() has undefined
+ * behaviour on sockets according to POSIX, so it doesn't
+ * work outside Linux.
+ */
+if ((pid = virFork()) 0)
+goto error;
+
+if (pid == 0) {
+umask(0077);
+if (bind(passfd, remoteAddr.data.sa, remoteAddr.len) 0)
+_exit(EXIT_FAILURE);
+
+_exit(EXIT_SUCCESS);
+}
+
+if (virProcessWait(pid, status, false) 0)
+goto error;
-retries++;
-usleep(1000 * 100 * retries);
+if (status != EXIT_SUCCESS) {
+/*
+ * OK, so the subprocces failed to bind() the socket. This may
mean
+ * that another daemon was starting at the same time and succeeded
+ * with its bind(). So we'll try connecting again, but this time
+ * without spawning the daemon.
+ */
+spawnDaemon = false;
goto retry;
}
-virReportSystemError(errno,
- _(Failed to connect socket to '%s'),
- path);
-goto error;
+if (listen(passfd, 0) 0) {
+virReportSystemError(errno, %s,
+ _(Failed to listen on socket that's about
+ to be passed to the daemon));
+goto error;
+}
+
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
+virReportSystemError(errno, _(Failed to connect socket to '%s'),
+ path);
+goto error;
+}
+
+if (virNetSocketForkDaemon(binary, passfd) 0)
+goto error;
}
localAddr.len = sizeof(localAddr.data);
@@ -603,7 +647,10 @@ int virNetSocketNewConnectUNIX(const char *path,
return 0;
error:
+VIR_FREE(buf);
VIR_FORCE_CLOSE(fd);
+if (spawnDaemon)
+unlink(path);
return -1;
}
#else
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
[libvirt] [PATCH v4 2/9] remote: create virNetServerServiceNewFDOrUNIX() wrapper
It's just a wrapper around NewFD and NewUNIX that selects the right
option and increments the number of used FDs.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetserverservice.c | 50 ++-
src/rpc/virnetserverservice.h | 14 +++-
3 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index d482a55..6b520b5 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -159,6 +159,7 @@ virNetServerServiceGetMaxRequests;
virNetServerServiceGetPort;
virNetServerServiceIsReadonly;
virNetServerServiceNewFD;
+virNetServerServiceNewFDOrUNIX;
virNetServerServiceNewPostExecRestart;
virNetServerServiceNewTCP;
virNetServerServiceNewUNIX;
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 320a02c..e85889b 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -1,7 +1,7 @@
/*
* virnetserverservice.c: generic network RPC server service
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2012, 2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -25,6 +25,8 @@
#include virnetserverservice.h
+#include unistd.h
+
#include viralloc.h
#include virerror.h
#include virthread.h
@@ -90,6 +92,52 @@ static void virNetServerServiceAccept(virNetSocketPtr sock,
}
+virNetServerServicePtr
+virNetServerServiceNewFDOrUNIX(const char *path,
+ mode_t mask,
+ gid_t grp,
+ int auth,
+#if WITH_GNUTLS
+ virNetTLSContextPtr tls,
+#endif
+ bool readonly,
+ size_t max_queued_clients,
+ size_t nrequests_client_max,
+ unsigned int nfds,
+ unsigned int *cur_fd)
+{
+if (*cur_fd - STDERR_FILENO nfds) {
+/*
+ * There are no more file descriptors to use, so we have to
+ * fallback to UNIX socket.
+ */
+return virNetServerServiceNewUNIX(path,
+ mask,
+ grp,
+ auth,
+#if WITH_GNUTLS
+ tls,
+#endif
+ readonly,
+ max_queued_clients,
+ nrequests_client_max);
+
+} else {
+/*
+ * There's still enough file descriptors. In this case we'll
+ * use the current one and increment it afterwards.
+ */
+return virNetServerServiceNewFD(*cur_fd++,
+auth,
+#if WITH_GNUTLS
+tls,
+#endif
+readonly,
+nrequests_client_max);
+}
+}
+
+
virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
const char *service,
int auth,
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index eb31abf..a1c8960 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -1,7 +1,7 @@
/*
* virnetserverservice.h: generic network RPC server service
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2011, 2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -37,6 +37,18 @@ typedef int
(*virNetServerServiceDispatchFunc)(virNetServerServicePtr svc,
virNetSocketPtr sock,
void *opaque);
+virNetServerServicePtr virNetServerServiceNewFDOrUNIX(const char *path,
+ mode_t mask,
+ gid_t grp,
+ int auth,
+# if WITH_GNUTLS
+ virNetTLSContextPtr tls,
+# endif
+ bool readonly,
+ size_t
max_queued_clients,
+ size_t
nrequests_client_max,
+ unsigned int nfds,
+ unsigned int *cur_fd);
virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
const char *service,
[libvirt] [PATCH v4 6/9] tests: support dynamic prefixes in commandtest
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
tests/commandtest.c | 49 ++---
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 7d2161c..ba823f7 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -62,7 +62,8 @@ main(void)
#else
-static int checkoutput(const char *testname)
+static int checkoutput(const char *testname,
+ char *prefix)
{
int ret = -1;
char *expectname = NULL;
@@ -86,6 +87,16 @@ static int checkoutput(const char *testname)
goto cleanup;
}
+if (prefix) {
+char *tmp = NULL;
+
+if (virAsprintf(tmp, %s%s, prefix, expectlog) 0)
+goto cleanup;
+
+VIR_FREE(expectlog);
+expectlog = tmp;
+}
+
if (STRNEQ(expectlog, actuallog)) {
virtTestDifference(stderr, expectlog, actuallog);
goto cleanup;
@@ -173,7 +184,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED)
return -1;
}
-if ((ret = checkoutput(test2)) != 0) {
+if ((ret = checkoutput(test2, NULL)) != 0) {
virCommandFree(cmd);
return ret;
}
@@ -187,7 +198,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test2);
+return checkoutput(test2, NULL);
}
/*
@@ -219,7 +230,7 @@ static int test3(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test3);
+ret = checkoutput(test3, NULL);
cleanup:
virCommandFree(cmd);
@@ -261,7 +272,7 @@ static int test4(const void *unused ATTRIBUTE_UNUSED)
while (kill(pid, 0) != -1)
usleep(100*1000);
-ret = checkoutput(test4);
+ret = checkoutput(test4, NULL);
cleanup:
virCommandFree(cmd);
@@ -291,7 +302,7 @@ static int test5(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test5);
+return checkoutput(test5, NULL);
}
@@ -315,7 +326,7 @@ static int test6(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test6);
+return checkoutput(test6, NULL);
}
@@ -340,7 +351,7 @@ static int test7(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test7);
+return checkoutput(test7, NULL);
}
/*
@@ -365,7 +376,7 @@ static int test8(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test8);
+return checkoutput(test8, NULL);
}
@@ -403,7 +414,7 @@ static int test9(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test9);
+return checkoutput(test9, NULL);
}
@@ -429,7 +440,7 @@ static int test10(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test10);
+return checkoutput(test10, NULL);
}
/*
@@ -453,7 +464,7 @@ static int test11(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test11);
+return checkoutput(test11, NULL);
}
/*
@@ -475,7 +486,7 @@ static int test12(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test12);
+return checkoutput(test12, NULL);
}
/*
@@ -510,7 +521,7 @@ static int test13(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test13);
+ret = checkoutput(test13, NULL);
cleanup:
virCommandFree(cmd);
@@ -582,7 +593,7 @@ static int test14(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test14);
+ret = checkoutput(test14, NULL);
cleanup:
virCommandFree(cmd);
@@ -613,7 +624,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test15);
+ret = checkoutput(test15, NULL);
cleanup:
VIR_FREE(cwd);
@@ -659,7 +670,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test16);
+ret = checkoutput(test16, NULL);
cleanup:
virCommandFree(cmd);
@@ -841,7 +852,7 @@ static int test20(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test20);
+ret = checkoutput(test20, NULL);
cleanup:
virCommandFree(cmd);
VIR_FREE(buf);
@@ -900,7 +911,7 @@ static int test21(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test21);
+ret = checkoutput(test21, NULL);
cleanup:
VIR_FREE(outbuf);
VIR_FREE(errbuf);
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 7/9] util: add virCommandPassListenFDs() function
That sets a new flag, but that flag does mean the child will get
LISTEN_FDS and LISTEN_PID environment variables properly set and
passed FDs reordered so that it corresponds with LISTEN_FDS (they must
start right after STDERR_FILENO).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/vircommand.c| 99
src/util/vircommand.h| 4 +-
tests/commanddata/test24.log | 7
tests/commandtest.c | 56 +
5 files changed, 166 insertions(+), 1 deletion(-)
create mode 100644 tests/commanddata/test24.log
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7516ed3..e09ddd5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1146,6 +1146,7 @@ virCommandNewArgList;
virCommandNewArgs;
virCommandNonblockingFDs;
virCommandPassFD;
+virCommandPassListenFDs;
virCommandRawStatus;
virCommandRequireHandshake;
virCommandRun;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index e775ba6..3b3e6f5 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -66,6 +66,7 @@ enum {
VIR_EXEC_CLEAR_CAPS = (1 2),
VIR_EXEC_RUN_SYNC = (1 3),
VIR_EXEC_ASYNC_IO = (1 4),
+VIR_EXEC_LISTEN_FDS = (1 5),
};
typedef struct _virCommandFD virCommandFD;
@@ -200,6 +201,78 @@ virCommandFDSet(virCommandPtr cmd,
return 0;
}
+static void
+virCommandReorderFDs(virCommandPtr cmd)
+{
+int maxfd = 0;
+int openmax = 0;
+size_t i = 0;
+
+if (!cmd || cmd-has_error || !cmd-npassfd)
+return;
+
+for (i = 0; i cmd-npassfd; i++)
+maxfd = MAX(cmd-passfd[i].fd, maxfd);
+
+openmax = sysconf(_SC_OPEN_MAX);
+if (openmax 0 ||
+maxfd + cmd-npassfd openmax)
+goto error;
+
+/*
+ * Simple two-pass sort, nothing fancy. This is not designed for
+ * anything else than passing around 2 FDs into the child.
+ *
+ * So first dup2() them somewhere else.
+ */
+for (i = 0; i cmd-npassfd; i++) {
+int newfd = maxfd + i + 1;
+int oldfd = cmd-passfd[i].fd;
+if (dup2(oldfd, newfd) != newfd) {
+virReportSystemError(errno,
+ _(Cannot dup2() fd %d before
+ passing it to the child),
+ oldfd);
+goto error;
+}
+VIR_FORCE_CLOSE(cmd-passfd[i].fd);
+}
+
+VIR_DEBUG(First reorder pass done);
+
+/*
+ * And then dup2() them in orderly manner.
+ */
+for (i = 0; i cmd-npassfd; i++) {
+int newfd = STDERR_FILENO + i + 1;
+int oldfd = maxfd + i + 1;
+if (dup2(oldfd, newfd) != newfd) {
+virReportSystemError(errno,
+ _(Cannot dup2() fd %d before
+ passing it to the child),
+ oldfd);
+goto error;
+}
+if (virSetInherit(newfd, true) 0) {
+virReportSystemError(errno,
+ _(Cannot set O_CLOEXEC on fd %d before
+ passing it to the child),
+ newfd);
+goto error;
+}
+VIR_FORCE_CLOSE(oldfd);
+cmd-passfd[i].fd = newfd;
+}
+
+VIR_DEBUG(Second reorder pass done);
+
+return;
+
+ error:
+cmd-has_error = -1;
+return;
+}
+
#ifndef WIN32
/**
@@ -678,6 +751,15 @@ virExec(virCommandPtr cmd)
goto fork_error;
}
+if (cmd-flags VIR_EXEC_LISTEN_FDS) {
+virCommandReorderFDs(cmd);
+virCommandAddEnvFormat(cmd, LISTEN_PID=%u, getpid());
+virCommandAddEnvFormat(cmd, LISTEN_FDS=%zu, cmd-npassfd);
+
+if (cmd-has_error)
+goto fork_error;
+}
+
/* Close logging again to ensure no FDs leak to child */
virLogReset();
@@ -919,6 +1001,23 @@ virCommandPassFD(virCommandPtr cmd, int fd, unsigned int
flags)
}
/**
+ * virCommandPassListenFDs:
+ * @cmd: the command to modify
+ *
+ * Pass LISTEN_FDS and LISTEN_PID environment variables into the
+ * child. LISTEN_PID has the value of the child's PID and LISTEN_FDS
+ * is a number of passed file descriptors starting from 3.
+ */
+void
+virCommandPassListenFDs(virCommandPtr cmd)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-flags |= VIR_EXEC_LISTEN_FDS;
+}
+
+/**
* virCommandSetPidFile:
* @cmd: the command to modify
* @pidfile: filename to use
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 8cdb31c..d3b286d 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -1,7 +1,7 @@
/*
* vircommand.h: Child command execution
*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General
Re: [libvirt] [PATCH] docs: fix missing forward slash
On 08/14/2014 12:17 PM, Jianwei Hu wrote: Should like below: interface type='server' mac address='52:54:00:22:c9:42'/ source address='192.168.0.1' port='5558'/ /interface ... interface type='client' mac address='52:54:00:8b:c9:51'/ source address='192.168.0.1' port='5558'/ /interface --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK and pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 5/9] cfg.mk: allow integers to be assigned a value computed with i|j|k
Even line like this: int asdf = i - somevar; was matched by the regex, so this patch adds '=' to the list of characters that break the regexp. Signed-off-by: Martin Kletzander mklet...@redhat.com --- cfg.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index 35ef444..eed8af5 100644 --- a/cfg.mk +++ b/cfg.mk @@ -568,7 +568,7 @@ sc_avoid_attribute_unused_in_header: $(_sc_search_regexp) sc_prohibit_int_ijk: - @prohibit='\(int|unsigned) ([^(]* )*(i|j|k)\(\s|,|;)' \ + @prohibit='\(int|unsigned) ([^(=]* )*(i|j|k)\(\s|,|;)'\ halt='use size_t, not int/unsigned int for loop vars i, j, k' \ $(_sc_search_regexp) -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 9/9] daemon: use socket activation with systemd
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
.gitignore | 1 +
daemon/Makefile.am | 14 --
daemon/libvirtd.conf | 5 +
daemon/libvirtd.service.in | 5 -
daemon/libvirtd.socket.in | 11 +++
libvirt.spec.in| 25 -
6 files changed, 49 insertions(+), 12 deletions(-)
create mode 100644 daemon/libvirtd.socket.in
diff --git a/.gitignore b/.gitignore
index 90fee91..9776ea1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -60,6 +60,7 @@
/daemon/libvirtd.pod
/daemon/libvirtd.policy
/daemon/libvirtd.service
+/daemon/libvirtd.socket
/daemon/test_libvirtd.aug
/docs/aclperms.htmlinc
/docs/apibuild.py.stamp
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 00221ab..70b7655 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -55,6 +55,7 @@ EXTRA_DIST = \
libvirtd.policy.in \
libvirtd.sasl \
libvirtd.service.in \
+ libvirtd.socket.in \
libvirtd.sysconf\
libvirtd.sysctl \
libvirtd.aug\
@@ -388,15 +389,18 @@ endif ! LIBVIRT_INIT_SCRIPT_UPSTART
if LIBVIRT_INIT_SCRIPT_SYSTEMD
SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system
-BUILT_SOURCES += libvirtd.service
+BUILT_SOURCES += libvirtd.service libvirtd.socket
-install-init-systemd: install-sysconfig libvirtd.service
+install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket
$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
$(INSTALL_DATA) libvirtd.service \
$(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
+ $(INSTALL_DATA) libvirtd.socket \
+ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
uninstall-init-systemd: uninstall-sysconfig
rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
+ rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || :
else ! LIBVIRT_INIT_SCRIPT_SYSTEMD
install-init-systemd:
@@ -420,6 +424,12 @@ libvirtd.service: libvirtd.service.in
$(top_builddir)/config.status
$ $@-t \
mv $@-t $@
+libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status
+ $(AM_V_GEN)sed \
+ -e 's|[@]runstatedir[@]|$(runstatedir)|g' \
+$ $@-t \
+ mv $@-t $@
+
check-local: check-augeas
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 2d80274..d4f6a1c 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -77,6 +77,11 @@
# UNIX socket access controls
#
+# Beware that if you are changing *any* of these options, and you use
+# socket activation with systemd, you need to adjust the settings in
+# the libvirtd.socket file as well since it could impose a security
+# risk if you rely on file permission checking only.
+
# Set the UNIX domain socket group ownership. This can be used to
# allow a 'trusted' set of users access to management capabilities
# without becoming root.
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index 086da36..1759ac8 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -1,8 +1,3 @@
-# NB we don't use socket activation. When libvirtd starts it will
-# spawn any virtual machines registered for autostart. We want this
-# to occur on every boot, regardless of whether any client connects
-# to a socket. Thus socket activation doesn't have any benefit
-
[Unit]
Description=Virtualization daemon
Before=libvirt-guests.service
diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in
new file mode 100644
index 000..0915bb3
--- /dev/null
+++ b/daemon/libvirtd.socket.in
@@ -0,0 +1,11 @@
+[Socket]
+ListenStream=@runstatedir@/libvirt/libvirt-sock
+ListenStream=@runstatedir@/libvirt/libvirt-sock-ro
+
+; The following settings must match libvirtd.conf file in order to
+; work as expected because libvirtd can't change them later.
+; SocketMode=0777 is safe only if authentication on the socket is set
+; up. For further information, please see the libvirtd.conf file.
+SocketMode=0777
+SocketUser=root
+SocketGroup=root
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 29da071..7fc6a8d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1670,11 +1670,13 @@ done
%if %{with_systemd}
%if %{with_systemd_macros}
-%systemd_post virtlockd.socket libvirtd.service
+%systemd_post virtlockd.socket libvirtd.service libvirtd.socket
%else
if [ $1 -eq 1 ] ; then
# Initial installation
-/bin/systemctl enable virtlockd.socket libvirtd.service /dev/null 21 ||
:
+/bin/systemctl enable \
+
[libvirt] [PATCH v4 1/9] util: abstract parsing of passed FDs into virGetListenFDs()
Since not only systemd can do this (we'll be doing it as well few
patches later), change 'systemd' to 'caller' and fix LISTEN_FDS to
LISTEN_PID where applicable.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_private.syms | 1 +
src/locking/lock_daemon.c | 45 +++---
src/util/virutil.c| 62 +++
src/util/virutil.h| 2 ++
4 files changed, 69 insertions(+), 41 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 08111d4..7516ed3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2107,6 +2107,7 @@ virGetGroupID;
virGetGroupList;
virGetGroupName;
virGetHostname;
+virGetListenFDs;
virGetSelfLastChanged;
virGetUnprivSGIOSysfsPath;
virGetUserCacheDirectory;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 3379f29..e9219d5 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -600,50 +600,13 @@ static int
virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
{
virNetServerServicePtr svc;
-const char *pidstr;
-const char *fdstr;
-unsigned long long procid;
unsigned int nfds;
-VIR_DEBUG(Setting up networking from systemd);
-
-if (!(pidstr = virGetEnvAllowSUID(LISTEN_PID))) {
-VIR_DEBUG(No LISTEN_FDS from systemd);
-return 0;
-}
-
-if (virStrToLong_ull(pidstr, NULL, 10, procid) 0) {
-VIR_DEBUG(Malformed LISTEN_PID from systemd %s, pidstr);
-return 0;
-}
-
-if ((pid_t)procid != getpid()) {
-VIR_DEBUG(LISTEN_PID %s is not for us %llu,
- pidstr, (unsigned long long)getpid());
-return 0;
-}
-
-if (!(fdstr = virGetEnvAllowSUID(LISTEN_FDS))) {
-VIR_DEBUG(No LISTEN_FDS from systemd);
-return 0;
-}
-
-if (virStrToLong_ui(fdstr, NULL, 10, nfds) 0) {
-VIR_DEBUG(Malformed LISTEN_FDS from systemd %s, fdstr);
-return 0;
-}
-
-if (nfds 1) {
-VIR_DEBUG(Too many (%d) file descriptors from systemd,
- nfds);
-nfds = 1;
-}
-
-unsetenv(LISTEN_PID);
-unsetenv(LISTEN_FDS);
-
-if (nfds == 0)
+if ((nfds = virGetListenFDs()) == 0)
return 0;
+if (nfds 1)
+VIR_DEBUG(Too many (%d) file descriptors from systemd, nfds);
+nfds = 1;
/* Systemd passes FDs, starting immediately after stderr,
* so the first FD we'll get is '3'. */
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 50faf2b..1d897d9 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2396,3 +2396,65 @@ void virUpdateSelfLastChanged(const char *path)
selfLastChanged = sb.st_ctime;
}
}
+
+/**
+ * virGetListenFDs:
+ *
+ * Parse LISTEN_PID and LISTEN_FDS passed from caller.
+ *
+ * Returns number of passed FDs.
+ */
+unsigned int
+virGetListenFDs(void)
+{
+const char *pidstr;
+const char *fdstr;
+size_t i = 0;
+unsigned long long procid;
+unsigned int nfds;
+
+VIR_DEBUG(Setting up networking from caller);
+
+if (!(pidstr = virGetEnvAllowSUID(LISTEN_PID))) {
+VIR_DEBUG(No LISTEN_PID from caller);
+return 0;
+}
+
+if (virStrToLong_ull(pidstr, NULL, 10, procid) 0) {
+VIR_DEBUG(Malformed LISTEN_PID from caller %s, pidstr);
+return 0;
+}
+
+if ((pid_t)procid != getpid()) {
+VIR_DEBUG(LISTEN_PID %s is not for us %llu,
+ pidstr, (unsigned long long)getpid());
+return 0;
+}
+
+if (!(fdstr = virGetEnvAllowSUID(LISTEN_FDS))) {
+VIR_DEBUG(No LISTEN_FDS from caller);
+return 0;
+}
+
+if (virStrToLong_ui(fdstr, NULL, 10, nfds) 0) {
+VIR_DEBUG(Malformed LISTEN_FDS from caller %s, fdstr);
+return 0;
+}
+
+unsetenv(LISTEN_PID);
+unsetenv(LISTEN_FDS);
+
+VIR_DEBUG(Got %u file descriptors, nfds);
+
+for (i = 0; i nfds; i++) {
+int fd = STDERR_FILENO + i + 1;
+
+VIR_DEBUG(Disabling inheritance of passed FD %d, fd);
+
+if (virSetInherit(fd, false) 0) {
+VIR_WARN(Couldn't disable inheritance of passed FD %d, fd);
+}
+}
+
+return nfds;
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index f93ea93..89b7923 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -232,4 +232,6 @@ typedef enum {
VIR_ENUM_DECL(virTristateBool)
VIR_ENUM_DECL(virTristateSwitch)
+unsigned int virGetListenFDs(void);
+
#endif /* __VIR_UTIL_H__ */
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/9] Series on passing FDs to daemon
On Thu, Aug 14, 2014 at 02:08:51PM +0200, Martin Kletzander wrote: v3 is here: https://www.redhat.com/archives/libvir-list/2014-July/msg01185.html the bug I'm still trying to fix is here: https://bugzilla.redhat.com/show_bug.cgi?id=927369 I forgot to mention that only patches 4, 8 and probably 9 need to be ACKed since v3. Martin Kletzander (9): util: abstract parsing of passed FDs into virGetListenFDs() remote: create virNetServerServiceNewFDOrUNIX() wrapper rpc: set listen backlog on FDs as well as on other sockets daemon: support passing FDs from the calling process cfg.mk: allow integers to be assigned a value computed with i|j|k tests: support dynamic prefixes in commandtest util: add virCommandPassListenFDs() function rpc: pass listen FD to the daemon being started daemon: use socket activation with systemd .gitignore| 1 + cfg.mk| 2 +- daemon/Makefile.am| 14 +- daemon/libvirtd.c | 45 ++ daemon/libvirtd.conf | 5 ++ daemon/libvirtd.service.in| 5 -- daemon/libvirtd.socket.in | 11 + libvirt.spec.in | 25 -- src/libvirt_private.syms | 2 + src/libvirt_remote.syms | 1 + src/locking/lock_daemon.c | 47 ++- src/rpc/virnetserverservice.c | 55 +- src/rpc/virnetserverservice.h | 15 +- src/rpc/virnetsocket.c| 85 ++ src/util/vircommand.c | 99 +++ src/util/vircommand.h | 4 +- src/util/virutil.c| 62 + src/util/virutil.h| 2 + tests/commanddata/test24.log | 7 +++ tests/commandtest.c | 105 ++ 20 files changed, 478 insertions(+), 114 deletions(-) create mode 100644 daemon/libvirtd.socket.in create mode 100644 tests/commanddata/test24.log -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/9] rpc: set listen backlog on FDs as well as on other sockets
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/locking/lock_daemon.c | 2 +-
src/rpc/virnetserverservice.c | 5 +
src/rpc/virnetserverservice.h | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index e9219d5..02d77e3 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -614,7 +614,7 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
#if WITH_GNUTLS
NULL,
#endif
- false, 1)))
+ false, 0, 1)))
return -1;
if (virNetServerAddService(srv, svc, NULL) 0) {
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index e85889b..fea05c3 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -133,6 +133,7 @@ virNetServerServiceNewFDOrUNIX(const char *path,
tls,
#endif
readonly,
+max_queued_clients,
nrequests_client_max);
}
}
@@ -265,6 +266,7 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
virNetTLSContextPtr tls,
#endif
bool readonly,
+size_t max_queued_clients,
size_t nrequests_client_max)
{
virNetServerServicePtr svc;
@@ -292,6 +294,9 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
goto error;
for (i = 0; i svc-nsocks; i++) {
+if (virNetSocketListen(svc-socks[i], max_queued_clients) 0)
+goto error;
+
/* IO callback is initially disabled, until we're ready
* to deal with incoming clients */
if (virNetSocketAddIOCallback(svc-socks[i],
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index a1c8960..b1d6c2d 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -74,6 +74,7 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
virNetTLSContextPtr tls,
# endif
bool readonly,
+size_t max_queued_clients,
size_t nrequests_client_max);
virNetServerServicePtr virNetServerServiceNewPostExecRestart(virJSONValuePtr
object);
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/9] Series on passing FDs to daemon
v3 is here: https://www.redhat.com/archives/libvir-list/2014-July/msg01185.html the bug I'm still trying to fix is here: https://bugzilla.redhat.com/show_bug.cgi?id=927369 Martin Kletzander (9): util: abstract parsing of passed FDs into virGetListenFDs() remote: create virNetServerServiceNewFDOrUNIX() wrapper rpc: set listen backlog on FDs as well as on other sockets daemon: support passing FDs from the calling process cfg.mk: allow integers to be assigned a value computed with i|j|k tests: support dynamic prefixes in commandtest util: add virCommandPassListenFDs() function rpc: pass listen FD to the daemon being started daemon: use socket activation with systemd .gitignore| 1 + cfg.mk| 2 +- daemon/Makefile.am| 14 +- daemon/libvirtd.c | 45 ++ daemon/libvirtd.conf | 5 ++ daemon/libvirtd.service.in| 5 -- daemon/libvirtd.socket.in | 11 + libvirt.spec.in | 25 -- src/libvirt_private.syms | 2 + src/libvirt_remote.syms | 1 + src/locking/lock_daemon.c | 47 ++- src/rpc/virnetserverservice.c | 55 +- src/rpc/virnetserverservice.h | 15 +- src/rpc/virnetsocket.c| 85 ++ src/util/vircommand.c | 99 +++ src/util/vircommand.h | 4 +- src/util/virutil.c| 62 + src/util/virutil.h| 2 + tests/commanddata/test24.log | 7 +++ tests/commandtest.c | 105 ++ 20 files changed, 478 insertions(+), 114 deletions(-) create mode 100644 daemon/libvirtd.socket.in create mode 100644 tests/commanddata/test24.log -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Redundant listen address entry in quest xml
On Thu, Aug 14, 2014 at 02:12:51PM +0200, Martin Kletzander wrote: On Thu, Aug 14, 2014 at 12:12:33PM +0100, Daniel P. Berrange wrote: On Thu, Aug 14, 2014 at 01:09:32PM +0200, Erik Skultety wrote: When editing guest's XML (on QEMU), it was possible to add multiple listen elements into graphics parent element. However QEMU does not support listening on multiple addresses. This patch causes qemu post-parse callback to remove any redundant entries, leaving only 1 listening address if provided, otherwise the configuration remains untouched. Discarding part of the users requested config like this is not the right thing todo. If the user requests multiple listen addresses and we cannot honour that request that we must report that as an error with code VIR_ERR_CONFIG_UNSUPPORTED But if we didn't failed parsing that before, domains with such XMLs will disappear from the list after daemon restart. Unless it is before starting the domain and not in PostParse, which is what you probably meant, sorry for my rapid response ;) Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Redundant listen address entry in quest xml
On Thu, Aug 14, 2014 at 12:12:33PM +0100, Daniel P. Berrange wrote:
On Thu, Aug 14, 2014 at 01:09:32PM +0200, Erik Skultety wrote:
When editing guest's XML (on QEMU), it was possible to add multiple
listen elements into graphics parent element. However QEMU does not
support listening on multiple addresses. This patch causes qemu
post-parse callback to remove any redundant entries, leaving only 1
listening address if provided, otherwise the configuration remains
untouched.
Discarding part of the users requested config like this is not
the right thing todo. If the user requests multiple listen addresses
and we cannot honour that request that we must report that as an
error with code VIR_ERR_CONFIG_UNSUPPORTED
But if we didn't failed parsing that before, domains with such XMLs
will disappear from the list after daemon restart.
---
src/qemu/qemu_domain.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4f63c88..75a4446 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -827,6 +827,25 @@ qemuDomainDefPostParse(virDomainDefPtr def,
VIR_DOMAIN_INPUT_BUS_USB) 0)
return -1;
+/* loop over all graphics connections and all listening addresses,
+ * removing all redundant listening address entries, thus leaving
+ * only 1 entry
+ */
+if (def-ngraphics 0 def-graphics) {
+size_t i, j;
+for (i = 0; i def-ngraphics; i++) {
+virDomainGraphicsListenDefPtr listens = def-graphics[i]-listens;
+size_t nListens = def-graphics[i]-nListens;
+if (nListens = 1 || !listens)
+continue;
+for (j = 1; j nListens; j++) {
+VIR_FREE(listens[j].address);
+VIR_FREE(listens[j].network);
+}
+def-graphics[i]-nListens = 1;
+}
+}
+
return 0;
}
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
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Redundant listen address entry in quest xml
On Thu, Aug 14, 2014 at 02:17:15PM +0200, Martin Kletzander wrote: On Thu, Aug 14, 2014 at 02:12:51PM +0200, Martin Kletzander wrote: On Thu, Aug 14, 2014 at 12:12:33PM +0100, Daniel P. Berrange wrote: On Thu, Aug 14, 2014 at 01:09:32PM +0200, Erik Skultety wrote: When editing guest's XML (on QEMU), it was possible to add multiple listen elements into graphics parent element. However QEMU does not support listening on multiple addresses. This patch causes qemu post-parse callback to remove any redundant entries, leaving only 1 listening address if provided, otherwise the configuration remains untouched. Discarding part of the users requested config like this is not the right thing todo. If the user requests multiple listen addresses and we cannot honour that request that we must report that as an error with code VIR_ERR_CONFIG_UNSUPPORTED But if we didn't failed parsing that before, domains with such XMLs will disappear from the list after daemon restart. Unless it is before starting the domain and not in PostParse, which is what you probably meant, sorry for my rapid response ;) Yes, we should be doing this at time of building QEMU cli args. 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] LXC: Fix virLXCControllerSetupDevPTS() wrt user namespaces
On 07/28/2014 10:59 PM, Richard Weinberger wrote:
The gid value passed to devpts has to be translated by hand as
virLXCControllerSetupDevPTS() is called before setting up the user
and group mappings.
Otherwise devpts will use an unmapped gid and openpty()
will fail within containers.
Linux commit commit 23adbe12
s/commit commit/kernel commit/
(fs,userns: Change inode_capable to capable_wrt_inode_uidgid)
uncovered that issue.
Signed-off-by: Richard Weinberger rich...@nod.at
---
src/lxc/lxc_controller.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 2d220eb..82ecf12 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1164,6 +1164,19 @@ static int virLXCControllerMain(virLXCControllerPtr
ctrl)
return rc;
}
+static uint32_t
I've changed this to 'unsigned int' to match the type used by
virDomainIdMapEntry.
+virLXCControllerLookupUsernsMap(virDomainIdMapEntryPtr map, int num,
+uint32_t src)
+{
+int i;
This should be size_t to pass 'make syntax-check'.
+
+for (i = 0; i num; i++) {
+if (src map[i].start src map[i].start + map[i].count)
+return map[i].target + (src - map[i].start);
+}
+
+return src;
+}
static int
virLXCControllerSetupUsernsMap(virDomainIdMapEntryPtr map,
ACK; pushed now.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] qemu: Redundant listen address entry in quest xml
When editing guest's XML (on QEMU), it was possible to add multiple
listen elements into graphics parent element. However QEMU does not
support listening on multiple addresses. Configuration is tested for
multiple 'listen address' and if positive, an error is raised.
---
src/qemu/qemu_process.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9e6a9ae..1810e6c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3934,6 +3934,14 @@ int qemuProcessStart(virConnectPtr conn,
}
graphics-listens[0].fromConfig = true;
}
+/* multiple listen addresses are unsupported configuration in qemu
+ */
+else if (graphics-nListens 1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(QEMU does not support multiple listen
+ addresses for a domain.));
+goto cleanup;
+}
}
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 1/3] qemu: Hide vram attribute for some useless cases.
From: Zeng Junliang zengjunli...@huawei.com The vram attribute is never used for VGA, CIRRUS and VMVGA as QEMU has no vram attribute for these models. Hide it for qemu in qemuDomainDevicePostParse function. And update the corresponding test cases and descriptions. Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- docs/formatdomain.html.in | 24 +++--- src/qemu/qemu_command.c| 3 ++- src/qemu/qemu_domain.c | 12 +++ ...qemuhotplug-console-compat-2+console-virtio.xml | 2 +- .../qemuxml2argv-console-compat-2.xml | 2 +- .../qemuxml2argv-controller-order.xml | 2 +- .../qemuxml2argv-graphics-listen-network.xml | 2 +- .../qemuxml2argv-graphics-listen-network2.xml | 2 +- .../qemuxml2argv-graphics-sdl-fullscreen.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +- .../qemuxml2argv-graphics-spice-agentmouse.xml | 2 +- .../qemuxml2argv-graphics-spice-timeout.xml| 2 +- .../qemuxml2argv-graphics-vnc-policy.xml | 2 +- .../qemuxml2argv-graphics-vnc-sasl.xml | 2 +- .../qemuxml2argv-graphics-vnc-socket.xml | 2 +- .../qemuxml2argv-graphics-vnc-tls.xml | 2 +- .../qemuxml2argv-graphics-vnc-websocket.xml| 2 +- .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 2 +- .../qemuxml2argv-net-bandwidth.xml | 2 +- .../qemuxml2argv-pci-autoadd-addr.xml | 2 +- .../qemuxml2argv-pci-autoadd-idx.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- .../qemuxml2xmlout-graphics-listen-network2.xml| 2 +- .../qemuxml2xmlout-graphics-spice-timeout.xml | 2 +- .../qemuxml2xmlout-pci-autoadd-addr.xml| 2 +- .../qemuxml2xmlout-pci-autoadd-idx.xml | 2 +- 27 files changed, 50 insertions(+), 37 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bd99ae0..3012e3c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4419,7 +4419,7 @@ qemu-kvm -net nic,model=? /dev/null ... lt;devicesgt; lt;videogt; - lt;model type='vga' vram='8192' heads='1'gt; + lt;model type='vga' heads='1'gt; lt;acceleration accel3d='yes' accel2d='yes'/gt; lt;/modelgt; lt;/videogt; @@ -4434,17 +4434,16 @@ qemu-kvm -net nic,model=? /dev/null is set but there is a codegraphics/code in domain xml, then libvirt will add a default codevideo/code according to the guest type. For a guest of type kvm, the default codevideo/code for it is: -codetype/code with value cirrus, codevram/code with value -9216, and codeheads/code with value 1. By default, the first -video device in domain xml is the primary one, but the optional -attribute codeprimary/code (span class=sincesince 1.0.2/span) -with value 'yes' can be used to mark the primary in cases of multiple -video device. The non-primary must be type of qxl. The optional -attribute coderam/code (span class=sincesince -1.0.2/span) is allowed for qxl type only and specifies -the size of the primary bar, while codevram/code specifies the -secondary bar size. If ram or vram are not supplied a default -value is used. +codetype/code with value cirrus, and codeheads/code with +value 1. By default, the first video device in domain xml is the +primary one, but the optional attribute codeprimary/code +(span class=sincesince 1.0.2/span) with value 'yes' can be +used to mark the primary in cases of multiple video device. +The non-primary must be type of qxl. The optional attribute +coderam/code (span class=sincesince 1.0.2/span) is allowed +for qxl type only and specifies the size of the primary bar, +while codevram/code specifies the secondary bar size. +If ram or vram are not supplied a default value is used. /dd dtcodemodel/code/dt @@ -4456,6 +4455,7 @@ qemu-kvm -net nic,model=? /dev/null You can also provide the amount of video memory in kibibytes (blocks of 1024 bytes) using codevram/code and the number of screen with codeheads/code. +For type of kvm codevram/code attribute is only valid for qxl. /dd dtcodeacceleration/code/dt diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a69976..c3f860e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11766,7 +11766,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, vid-type = VIR_DOMAIN_VIDEO_TYPE_XEN; else vid-type = video; -vid-vram = virDomainVideoDefaultRAM(def, vid-type); +vid-vram =
[libvirt] [PATCH V3 3/3] qemu: Add secondary-vga support
From: Zeng Junliang zengjunli...@huawei.com
Secondary-vga is supported by QEMU in currently master. Add it supported
in libvirt as qemu commandline show: '-device secondary-vga'. And it can
be used as secondary display device, like qxl. Also, add test cases and
descriptions for it.
Libvirt xml configuration sample:
video
model type='secondary' vgamem='16384' heads='1'/
/video
The resulting qemu command line change is the addition of:
-device secondary-vga,id=video0,vgamem_mb=16,bus=pci.0,addr=0x5
Signed-off-by: Zeng Junliang zengjunli...@huawei.com
Signed-off-by: Wang Rui moon.wang...@huawei.com
---
docs/formatdomain.html.in | 13 +++---
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 9 ++--
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c| 54 +++---
.../qemuxml2argv-graphics-vnc-secondary-vga.args | 7 +++
.../qemuxml2argv-graphics-vnc-secondary-vga.xml| 39
tests/qemuxml2argvtest.c | 4 ++
tests/qemuxml2xmltest.c| 1 +
11 files changed, 106 insertions(+), 26 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a0d15c4..b1d8ad4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4440,9 +4440,9 @@ qemu-kvm -net nic,model=? /dev/null
device in domain xml is the primary one, but the optional attribute
codeprimary/code (span class=sincesince 1.0.2/span) with
value 'yes' can be used to mark the primary in cases of multiple video
-device. The non-primary must be type of qxl. The optional attribute
-coderam/code (span class=sincesince 1.0.2/span) is allowed
-for qxl type only and specifies the size of the primary bar,
+device. The non-primary must be type of qxl or secondary. The
+optional attribute coderam/code (span class=sincesince
1.0.2/span)
+is allowed for qxl type only and specifies the size of the primary
bar,
while codevram/code specifies the secondary bar size.
If ram, vram or vgamem are not supplied a default value is used.
/dd
@@ -4451,8 +4451,9 @@ qemu-kvm -net nic,model=? /dev/null
dd
The codemodel/code element has a mandatory codetype/code
attribute which takes the value vga, cirrus, vmvga, xen,
-vbox, or qxl (span class=sincesince 0.8.6/span)
-depending on the hypervisor features available.
+vbox, qxl (span class=sincesince 0.8.6/span) or
+secondary (span class=sincesince 1.2.8/span) depending
+on the hypervisor features available.
p
codevram/code attribute specifies the amount of video memory
in kibibytes (blocks of 1024 bytes). For type of kvm, it is only
@@ -4462,7 +4463,7 @@ qemu-kvm -net nic,model=? /dev/null
codevgamem/code attribute span class=sincesince 1.2.8,
QEMU and KVM only/span specifies the size of the framebuffer
portion of the ram region. And it is only valid for type of
-vga, vmvga and qxl.
+vga, vmvga, qxl and secondary.
/p
p
codeheads/code attribute specifies the number of screen.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b2cc218..db8dbde 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2768,6 +2768,7 @@
valuevmvga/value
valuexen/value
valuevbox/value
+valuesecondary/value
/choice
/attribute
group
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7097570..c7b9a5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -490,7 +490,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
vmvga,
xen,
vbox,
- qxl)
+ qxl,
+ secondary)
VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST,
mouse,
@@ -9646,7 +9647,8 @@ virDomainVideoDefaultVgamem(int type)
case VIR_DOMAIN_VIDEO_TYPE_VGA:
case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
case VIR_DOMAIN_VIDEO_TYPE_QXL:
-/* QEMU use 16M as default value for vga/vmvga/qxl device*/
+case VIR_DOMAIN_VIDEO_TYPE_SECONDARY:
+/* QEMU use 16M as default value for vga/vmvga/qxl/secondary device */
return 16 * 1024;
default:
@@ -9781,7 +9783,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
if (vgamem) {
if (def-type !=
[libvirt] [PATCH V3 0/3] Some improvements for video model
From: Zeng Junliang zengjunli...@huawei.com http://www.redhat.com/archives/libvir-list/2014-July/msg00644.html diff to v2: - hide vram attribute silently instead of reporting an error. - introduce three new capabilities for vga.vgamem_mb, vmvga.vgamem_mb and qxl.vgamem_mb. - fix some error reported by building libvirt. Zeng Junliang (3): qemu: Hide vram attribute for some useless cases. qemu: Introduce vgamem attribute for video model qemu: Add secondary-vga support docs/formatdomain.html.in | 46 +++--- docs/schemas/domaincommon.rng | 6 + src/conf/domain_conf.c | 57 +++- src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 17 +++ src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_command.c| 162 +++-- src/qemu/qemu_domain.c | 12 ++ tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 3 + tests/qemuhelptest.c | 10 +- ...qemuhotplug-console-compat-2+console-virtio.xml | 2 +- .../qemuxml2argv-console-compat-2.xml | 2 +- .../qemuxml2argv-controller-order.xml | 2 +- .../qemuxml2argv-graphics-listen-network.xml | 2 +- .../qemuxml2argv-graphics-listen-network2.xml | 2 +- .../qemuxml2argv-graphics-sdl-fullscreen.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +- ...emuxml2argv-graphics-spice-agent-file-xfer.args | 5 +- ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 4 +- .../qemuxml2argv-graphics-spice-agentmouse.xml | 2 +- .../qemuxml2argv-graphics-spice-compression.args | 4 +- .../qemuxml2argv-graphics-spice-compression.xml| 4 +- .../qemuxml2argv-graphics-spice-listen-network.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml| 4 +- .../qemuxml2argv-graphics-spice-sasl.args | 3 +- .../qemuxml2argv-graphics-spice-sasl.xml | 2 +- .../qemuxml2argv-graphics-spice-timeout.xml| 2 +- .../qemuxml2argv-graphics-spice.args | 5 +- .../qemuxml2argv-graphics-spice.xml| 4 +- .../qemuxml2argv-graphics-vnc-policy.xml | 2 +- .../qemuxml2argv-graphics-vnc-sasl.xml | 2 +- .../qemuxml2argv-graphics-vnc-secondary-vga.args | 7 + .../qemuxml2argv-graphics-vnc-secondary-vga.xml| 39 + .../qemuxml2argv-graphics-vnc-socket.xml | 2 +- .../qemuxml2argv-graphics-vnc-std-vga.args | 4 + .../qemuxml2argv-graphics-vnc-std-vga.xml | 36 + .../qemuxml2argv-graphics-vnc-tls.xml | 2 +- .../qemuxml2argv-graphics-vnc-vmware-svga.args | 4 + .../qemuxml2argv-graphics-vnc-vmware-svga.xml | 36 + .../qemuxml2argv-graphics-vnc-websocket.xml| 2 +- .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 2 +- .../qemuxml2argv-net-bandwidth.xml | 2 +- .../qemuxml2argv-pci-autoadd-addr.xml | 2 +- .../qemuxml2argv-pci-autoadd-idx.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 2 +- .../qemuxml2argv-pcihole64-q35.args| 3 +- .../qemuxml2argv-pcihole64-q35.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-q35.xml| 2 +- .../qemuxml2argv-serial-spiceport.args | 4 +- .../qemuxml2argv-serial-spiceport.xml | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 9 +- .../qemuxml2argv-video-device-pciaddr-default.xml | 6 +- tests/qemuxml2argvtest.c | 26 +++- .../qemuxml2xmlout-graphics-listen-network2.xml| 2 +- .../qemuxml2xmlout-graphics-spice-timeout.xml | 2 +- .../qemuxml2xmlout-pci-autoadd-addr.xml| 2 +- .../qemuxml2xmlout-pci-autoadd-idx.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml| 2 +- tests/qemuxml2xmltest.c| 3 + 68 files changed, 494 insertions(+), 121 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-std-vga.args create mode 100644
[libvirt] [PATCH V3 2/3] qemu: Introduce vgamem attribute for video model
From: Zeng Junliang zengjunli...@huawei.com QEMU suopports to specifie the size of the framebuffer portion of the ram region for vga, vmvga and qxl through the qemu command line parameter vgamem_mb. This patch introduces vgamem attribute for video model, makes the vgamem_mb value configured in libvirt xml. Also, add test cases and descriptions for it. Libvirt xml configuration sample(based on VGA): video model type='vga' vgamem='16384' heads='1'/ /video The resulting qemu command line change is the addition of: -vga std -global VGA.vgamem_mb=16 or -device VGA,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2 If 'vgamem' is not configured by user, a default value will be assigned by libvirt. Assume that user wants to start a VM without 'vgamem' configuration in xml, if QEMU lacks the ability of vgamem, libvirt will only report a warning rather than make starting failed even though libvirt has assigned a default value to vgamem. It could avoid a regression. It's less confusing to introduce the new vgamem attribute. Prior to the change: model libvirt-attribute(xml) qemu-attribute qxl vramvram_size qxl nonevgamem_mb vga vramQEMU has no attribute named vram* vga nonevgamem_mb vmvga vramQEMU has no attribute named vram* vmvga nonevgamem_mb After the change: model libvirt attribute(xml) QEMU attribute qxl vramvram_size qxl vgamem vgamem_mb vga vramQEMU has no attribute named vram* vga vgamem vgamem_mb vmvga vramQEMU has no attribute named vram* vmvga vgamem vgamem_mb Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- docs/formatdomain.html.in | 35 --- docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 52 +- src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 15 +++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c| 105 - tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 3 + tests/qemuhelptest.c | 10 +- .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +- ...emuxml2argv-graphics-spice-agent-file-xfer.args | 5 +- ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 4 +- .../qemuxml2argv-graphics-spice-compression.args | 4 +- .../qemuxml2argv-graphics-spice-compression.xml| 4 +- .../qemuxml2argv-graphics-spice-listen-network.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml| 4 +- .../qemuxml2argv-graphics-spice-sasl.args | 3 +- .../qemuxml2argv-graphics-spice-sasl.xml | 2 +- .../qemuxml2argv-graphics-spice-timeout.xml| 2 +- .../qemuxml2argv-graphics-spice.args | 5 +- .../qemuxml2argv-graphics-spice.xml| 4 +- .../qemuxml2argv-graphics-vnc-std-vga.args | 4 + .../qemuxml2argv-graphics-vnc-std-vga.xml | 36 +++ .../qemuxml2argv-graphics-vnc-vmware-svga.args | 4 + .../qemuxml2argv-graphics-vnc-vmware-svga.xml | 36 +++ .../qemuxml2argv-net-bandwidth.xml | 2 +- .../qemuxml2argv-pcihole64-q35.args| 3 +- .../qemuxml2argv-pcihole64-q35.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-q35.xml| 2 +- .../qemuxml2argv-serial-spiceport.args | 4 +- .../qemuxml2argv-serial-spiceport.xml | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 9 +- .../qemuxml2argv-video-device-pciaddr-default.xml | 6 +- tests/qemuxml2argvtest.c | 22 - .../qemuxml2xmlout-graphics-spice-timeout.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml| 2 +- tests/qemuxml2xmltest.c| 2 + 45 files changed, 357 insertions(+), 77 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-std-vga.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-std-vga.xml create mode 100644
Re: [libvirt] [PATCH] LXC: Fix virLXCControllerSetupDevPTS() wrt user namespaces
Am 14.08.2014 14:35, schrieb Ján Tomko:
On 07/28/2014 10:59 PM, Richard Weinberger wrote:
The gid value passed to devpts has to be translated by hand as
virLXCControllerSetupDevPTS() is called before setting up the user
and group mappings.
Otherwise devpts will use an unmapped gid and openpty()
will fail within containers.
Linux commit commit 23adbe12
s/commit commit/kernel commit/
(fs,userns: Change inode_capable to capable_wrt_inode_uidgid)
uncovered that issue.
Signed-off-by: Richard Weinberger rich...@nod.at
---
src/lxc/lxc_controller.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 2d220eb..82ecf12 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1164,6 +1164,19 @@ static int virLXCControllerMain(virLXCControllerPtr
ctrl)
return rc;
}
+static uint32_t
I've changed this to 'unsigned int' to match the type used by
virDomainIdMapEntry.
Why is uint32_t wrong? :)
+virLXCControllerLookupUsernsMap(virDomainIdMapEntryPtr map, int num,
+uint32_t src)
+{
+int i;
This should be size_t to pass 'make syntax-check'.
/me pushes 'make syntax-check' to TODO list.
Thanks,
//richard
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: Fix virLXCControllerSetupDevPTS() wrt user namespaces
On 08/14/2014 02:45 PM, Richard Weinberger wrote: Am 14.08.2014 14:35, schrieb Ján Tomko: On 07/28/2014 10:59 PM, Richard Weinberger wrote: The gid value passed to devpts has to be translated by hand as virLXCControllerSetupDevPTS() is called before setting up the user and group mappings. Otherwise devpts will use an unmapped gid and openpty() will fail within containers. Linux commit commit 23adbe12 s/commit commit/kernel commit/ (fs,userns: Change inode_capable to capable_wrt_inode_uidgid) uncovered that issue. Signed-off-by: Richard Weinberger rich...@nod.at --- src/lxc/lxc_controller.c | 25 +++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 2d220eb..82ecf12 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1164,6 +1164,19 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl) return rc; } +static uint32_t I've changed this to 'unsigned int' to match the type used by virDomainIdMapEntry. Why is uint32_t wrong? :) Not really wrong, uint32_t should have the same range as unsigned int on Linux. I just wanted them to be consistent. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: Redundant listen address entry in quest xml
On 08/14/2014 02:44 PM, Erik Skultety wrote:
When editing guest's XML (on QEMU), it was possible to add multiple
listen elements into graphics parent element. However QEMU does not
support listening on multiple addresses. Configuration is tested for
multiple 'listen address' and if positive, an error is raised.
We have a public bug open for this issue. It's nice to add those in the commit
message, if someone wants to know why the commit was added in the future.
https://bugzilla.redhat.com/show_bug.cgi?id=1119212
---
src/qemu/qemu_process.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9e6a9ae..1810e6c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3934,6 +3934,14 @@ int qemuProcessStart(virConnectPtr conn,
}
graphics-listens[0].fromConfig = true;
}
+/* multiple listen addresses are unsupported configuration in
qemu
+ */
This comment is redundant - it basically repeats the error message.
+else if (graphics-nListens 1) {
We prefer putting 'else' on the same line as the closing brace of the 'if'
block:
http://libvirt.org/hacking.html#curly_braces
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(QEMU does not support multiple listen
+ addresses for a domain.));
This is not true. For example: you can use one VNC and one SPICE graphics,
both with different listen addresses. How about 'QEMU does not support
multiple listen addresses for one graphics device'?
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Maximum vlanid should be 4095 in interface.rng
On 08/13/2014 10:10 AM, Jianwei Hu wrote:
The correct vlanid range is 0~4095.
After merging this patch, we can not validate a interface xml with vlanid =
4096.
[root@localhost ~]# cat vlan.xml
interface type='vlan' name='eno1.4096'
start mode='onboot'/
protocol family='ipv4'
dhcp/
/protocol
vlan tag='4096'
interface name='eno1'/
/vlan
/interface
[root@localhost ~]# virt-xml-validate vlan.xml
vlan.xml:1: element interface: Relax-NG validity error : Invalid sequence in
interleave
vlan.xml:6: element vlan: Relax-NG validity error : Element interface failed
to validate content
vlan.xml:6: element vlan: Relax-NG validity error : Element vlan failed to
validate attributes
vlan.xml fails to validate
[root@localhost ~]#
Here is a ip command help on this.
[root@localhost /]# ip link add link eno1 name eno1.90 type vlan help
Usage: ... vlan [ protocol VLANPROTO ] id VLANID[ FLAG-LIST ]
[ ingress-qos-map QOS-MAP ] [ egress-qos-map QOS-MAP ]
VLANPROTO: [ 802.1Q / 802.1ad ]
VLANID := 0-4095
FLAG-LIST := [ FLAG-LIST ] FLAG
FLAG := [ reorder_hdr { on | off } ] [ gvrp { on | off } ] [ mvrp { on | off
} ]
[ loose_binding { on | off } ]
QOS-MAP := [ QOS-MAP ] QOS-MAPPING
QOS-MAPPING := FROM:TO
---
docs/schemas/interface.rng | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng
index 80962d4..0f577d6 100644
--- a/docs/schemas/interface.rng
+++ b/docs/schemas/interface.rng
@@ -440,7 +440,7 @@
define name='vlan-id'
data type=unsignedInt
- param name=maxInclusive4096/param
+ param name=maxInclusive4095/param
/data
/define
/grammar
ACK and pushed.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] [RFC] net: don't segfault on Martin's setup
No, don't worry, this is not a real commit message, this is more like
an RFC about how to fix it. I'm not sure about this part of libvirt
codebase, so I'm rather asking for help. Apparently, I have a network
with netdef-forward.pfs == NULL, I suspect this is a network I have,
that doesn't have any forward/ element.
With the following fix, everything works as expected, but I'm almost
certain this is not enough.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/network/bridge_driver.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1ba4c3d..3a40124 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2196,6 +2196,9 @@ networkCreateInterfacePool(virNetworkDefPtr netdef)
int ret = -1;
size_t i;
+if (!netdef-forward.pfs)
+return 0;
+
if ((virNetDevGetVirtualFunctions(netdef-forward.pfs-dev,
vfNames, virtFns, numVirtFns)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V5 04/12] src/xenxs: Refactor code formating virtual time config
On Wed, Aug 13, 2014 at 05:15:58PM -0600, Jim Fehlig wrote:
Kiarie Kahurani wrote:
introduce function
xenFormatXMTimeOffset(virConfPtr conf,);
which formats time config instead
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 153
-
1 file changed, 82 insertions(+), 71 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index ad70b5a..b17574d 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1704,6 +1704,86 @@ xenFormatXMMem(virConfPtr conf, virDomainDefPtr def)
}
+static int
+xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
If function name + params exceeds 80 columns, common practice is each
param after the first on a separate line. E.g.
xenFormatTimeOffset(virConfPtr conf,
virDomainDefPtr def,
int xendConfigVersion)
{
...
+{
+int vmlocaltime;
+if (xendConfigVersion XEND_CONFIG_VERSION_3_1_0) {
Preferred style is a blank line after local variable declaration.
ACK otherwise. I'll fixup these nits before pushing.
Regards,
Jim
BTW in the tests data I see configs like
clock offset = 'utc' adjustment = 'reset'
while if am not wrong the code in xen_xm.c explicity rejects such
config
unsupported clock adjustment = 'reset'
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V5 08/12] src/xenxs: Refactor code formating CPU features config
On Wed, Aug 13, 2014 at 05:35:52PM -0600, Jim Fehlig wrote:
Kiarie Kahurani wrote:
introduce functions
xenFormatXMCPUFeatures(virConfPtr conf, ..);
which formats CPU features config instead
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 118
-
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-pci-devs.cfg | 6 +-
27 files changed, 146 insertions(+), 128 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 2c381f8..a856698 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1950,6 +1950,73 @@ xenFormatXMDomainDisks(virConfPtr conf,
virDomainDefPtr def,
virConfFreeValue(diskVal);
return -1;
}
+
+
+static int
+xenFormatXMCPUFeatures(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+char *cpus = NULL;
+size_t i;
+
+if (xenXMConfigSetInt(conf, vcpus, def-maxvcpus) 0)
+return -1;
+/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
+ either 32, or 64 on a platform where long is big enough. */
+if (def-vcpus def-maxvcpus
+xenXMConfigSetInt(conf, vcpu_avail, (1UL def-vcpus) - 1) 0)
+return -1;
+
+if ((def-cpumask != NULL)
+((cpus = virBitmapFormat(def-cpumask)) == NULL)) {
+return -1;
+}
+
+if (cpus
+xenXMConfigSetString(conf, cpus, cpus) 0)
+return -1;
+
+VIR_FREE(cpus);
I'd consider these to be allocation-related, as opposed to features.
I've changed the patch a bit to introduce xenFormatXMCPUAllocation and
xenFormatXMCPUFeatures.
+if (STREQ(def-os.type, hvm)) {
+if (xenXMConfigSetInt(conf, pae,
+ (def-features[VIR_DOMAIN_FEATURE_PAE] ==
+ VIR_TRISTATE_SWITCH_ON) ? 1 : 0) 0)
+return -1;
+
+if (xenXMConfigSetInt(conf, acpi,
+ (def-features[VIR_DOMAIN_FEATURE_ACPI] ==
+ VIR_TRISTATE_SWITCH_ON) ? 1 : 0) 0)
+return -1;
+
+if (xenXMConfigSetInt(conf, apic,
+ (def-features[VIR_DOMAIN_FEATURE_APIC] ==
+ VIR_TRISTATE_SWITCH_ON) ? 1 : 0) 0)
+return -1;
+
+if (xendConfigVersion = XEND_CONFIG_VERSION_3_0_4) {
+if (xenXMConfigSetInt(conf, hap,
+ (def-features[VIR_DOMAIN_FEATURE_HAP] ==
+ VIR_TRISTATE_SWITCH_ON) ? 1 : 0) 0)
+return -1;
+
+if (xenXMConfigSetInt(conf, viridian,
+
(def-features[VIR_DOMAIN_FEATURE_VIRIDIAN] ==
+ VIR_TRISTATE_SWITCH_ON) ? 1 : 0) 0)
+return -1;
+}
+
+for (i = 0; i def-clock.ntimers; i++) {
+if (def-clock.timers[i]-name == VIR_DOMAIN_TIMER_NAME_HPET
+def-clock.timers[i]-present != -1
+xenXMConfigSetInt(conf, hpet,
def-clock.timers[i]-present) 0)
+return -1;
+}
+}
+
+return 0;
+}
+
+
/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
Re: [libvirt] [PATCH V5 09/12] src/xenxs: Refactor code formating OS config
On Thu, Aug 14, 2014 at 12:15:32AM -0600, Jim Fehlig wrote:
Kiarie Kahurani wrote:
introduce function
xenFormatXMOS(virConfPtr conf,);
I split this into three functions:
- xenFormatXMEmulator
- xenFormatXMCDROM
- xenFormatXMOS
Formating of emulator and cdrom can be different between xm and xl, and
we may want to account for that in the near future.
which formats OS config instead
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 95
--
tests/xmconfigdata/test-escape-paths.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-force-hpet.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-force-nohpet.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-localtime.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-net-ioemu.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-net-netfront.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-new-cdrom.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-old-cdrom.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-parallel-tcp.cfg | 2 +-
.../test-fullvirt-serial-dev-2-ports.cfg | 2 +-
.../test-fullvirt-serial-dev-2nd-port.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-file.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-null.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-pipe.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-serial-pty.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-stdio.cfg | 2 +-
.../test-fullvirt-serial-tcp-telnet.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-tcp.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-udp.cfg| 2 +-
tests/xmconfigdata/test-fullvirt-serial-unix.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-sound.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-utc.cfg | 2 +-
tests/xmconfigdata/test-no-source-cdrom.cfg| 2 +-
tests/xmconfigdata/test-pci-devs.cfg | 2 +-
Avoids changing all the test data files too. Simplified patch below.
BTW, I got interrupted during my earlier review, so will push shortly
had changed to will push tomorrow :-).
Regards,
Jim
From 76824b192ecac2d9f19406c11446b616ca96e9d4 Mon Sep 17 00:00:00 2001
From: Kiarie Kahurani davidkiar...@gmail.com
Date: Tue, 12 Aug 2014 00:21:32 +0300
Subject: [PATCH 08/11] src/xenxs: Refactor code formating OS config
introduce functions
xenFormatXMEmulator(virConfPtr conf,);
xenFormatXMCDROM(virConfPtr conf, ...);
xenFormatXMOS(virConfPtr conf,);
which formats OS and associated config instead
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/xenxs/xen_xm.c | 161
+
1 file changed, 101 insertions(+), 60 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 432fee2..72ae913 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -2021,42 +2021,57 @@ xenFormatXMCPUFeatures(virConfPtr conf,
}
-/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
- either 32, or 64 on a platform where long is big enough. */
-verify(MAX_VIRT_CPUS = sizeof(1UL) * CHAR_BIT);
+static int
+xenFormatXMEmulator(virConfPtr conf, virDomainDefPtr def)
+{
+if (def-emulator
+xenXMConfigSetString(conf, device_model, def-emulator) 0)
+return -1;
-virConfPtr
-xenFormatXM(virConnectPtr conn,
-virDomainDefPtr def,
-int xendConfigVersion)
+return 0;
+}
+
+
+static int
+xenFormatXMCDROM(virConfPtr conf,
+ virDomainDefPtr def,
+ int xendConfigVersion)
{
-virConfPtr conf = NULL;
-int hvm = 0;
size_t i;
-virConfValuePtr netVal = NULL;
-
-if (!(conf = virConfNew()))
-goto cleanup;
-if (xenFormatXMGeneralMeta(conf, def) 0)
-goto cleanup;
+if (STREQ(def-os.type, hvm)) {
+if (xendConfigVersion == XEND_CONFIG_VERSION_3_0_2) {
+for (i = 0; i def-ndisks; i++) {
+if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_CDROM
+def-disks[i]-dst
+STREQ(def-disks[i]-dst, hdc)
+virDomainDiskGetSource(def-disks[i])) {
+if (xenXMConfigSetString(conf, cdrom,
+
virDomainDiskGetSource(def-disks[i])) 0)
+return -1;
+break;
+}
+}
+}
+}
-if (xenFormatXMMem(conf, def) 0)
-goto cleanup;
+return 0;
+}
-if (xenFormatXMCPUAllocation(conf, def) 0)
-goto
Re: [libvirt] [PATCH] [RFC] net: don't segfault on Martin's setup
On 08/14/2014 10:13 AM, Martin Kletzander wrote: No, don't worry, this is not a real commit message, this is more like an RFC about how to fix it. I'm not sure about this part of libvirt codebase, so I'm rather asking for help. Apparently, I have a network with netdef-forward.pfs == NULL, I suspect this is a network I have, that doesn't have any forward/ element. Ouch! Actually it doesn't even need to be that extreme, it just needs to not have any pf elements. With the following fix, everything works as expected, but I'm almost certain this is not enough. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/network/bridge_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1ba4c3d..3a40124 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2196,6 +2196,9 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) int ret = -1; size_t i; +if (!netdef-forward.pfs) +return 0; + This will work, but I notice that all previously existing calls to networkCreateInterfacePool() check for npfs 0 and nifs == 0 prior to calling, so I'll make a patch that removes those checks from the callers and moves them into networkCreateInterfacePool(). Sorry for the mess, and thanks for debugging the failure. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] questions on backing chain file, block lun, active commit
On 13/08/14 19:48 -0600, Eric Blake wrote: While investigating active commit, I noticed that our code for probing backing chains currently guesses at type=file vs. type=block solely based on stat results (regular file vs. block device). Is a block device allowed to be used as disk type='block' device='lun' at the same time it has qcow2 format, or does use of device='lun' enforce that the block device is used with raw format? My worry is that if a lun device can be used with qcow2 format, then it can have a non-block backing file, and an active commit would convert disk type='block' device='lun' into disk type='file' device='lun' which is not valid. On a related vein, that means that an active commit currently auto-picks whether the disk will be listed as type='file' or type='block' solely on the stat results. Elsewhere, we allow type='file' even for block devices, where a noticeable difference is how virDomainGetBlockInfo() reports allocation (for type='file', allocation is based on how sparse the file is, but a block device is not sparse; for type='block', allocation is based on asking qemu the maximum used cluster). Should we be auto-converting to type='block' in other cases where we have stat results? For example, I posted a patch today to let the user explicitly request that virsh blockcopy to a block device will use type='block' because the existing code has always used type='file', but if we were doing things automatically, then the user would automatically get type='block' for block device destinations instead of having to request it; but has no way to forcefully list a file even when the destination is a block (at least, not until I implement virDomainBlockCopy() that takes an XML disk description). One drawback of autoconversion is the XML changes - with type='file', the host resource is located at xpath disk/source/@file, but with type='block', it is at xpath disk/source/@dev. If we ever convert a user's type='file' into a block device, but the user isn't expecting the conversion, then they won't be able to find the source file name in the output XML. So on that ground, autoprobing type='block' for active commit is fishy; we really should be honoring the user's input for backingStore rather than probing it ourselves every time. Until that point, and given that I just proposed a patch to turn on type='block' for blockcopy, where type='file' is used in all other cases, should we have the same sort of flag for active commit? Would the following semantics be desirable with respect to disk type conversion? We introduce flag(s) to permit type conversion from file-block (do we need one for the other way too)? For active commit, if one of these flags is not specified, no conversion will be done. If the parent volume type is not the same as the active layer then an error will be trigerred. In other words, the user is responsible for indicating which way a conversion should go if the types are different. -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/66] vbox: Rewrite vbox domain driver
Le Wednesday 13 August 2014 17:18:44, Michal Privoznik a écrit :
[CC-ing Yohan BELLEGUIC and Manuel VIVES]
On 12.08.2014 17:31, Michal Privoznik wrote:
On 11.08.2014 12:06, Taowei wrote:
This series of patches rewrite the vbox's domain
driver. The driver is separated into two parts: the version
specified and the common part. The common driver use
vboxUniformedAPI to build a general driver for all vbox
versions. The vboxUniformedAPI take the responsiblity to
communicate with virtualbox. Since there are some incompatible
changes in virtualbox, vboxUniformedAPI should be aware of
these changes and provide a uniformed api for the upper layer.
The significant result of this patch is that we replace all
vir${vbox_version}Driver into one virCommonDriver. So, we will
have only one vbox driver implementation for all vbox versions
in libvirt.
PS: I have send part of my patches before:
https://www.redhat.com/archives/libvir-list/2014-July/msg00937.html
But I have to resend it beacuse I did some improvement on previous
patches:
*Remove the test case for vboxUniformedAPI, because it would raise
break strict-aliasing rules warning in some distibutions
*Merged the flag fdWatchNeedInitialize into domainEventCallbacks,
So, we use one flag to indicate whether vbox support callbacks
as well as we need to initialize variables for it.
Taowei (66):
vbox: Begin to rewrite, vboxConnectOpen
vbox: Rewrite vboxConnectClose
vbox: Rewrite vboxDomainSave
vbox: Rewrite vboxConnectGetVersion
vbox: Rewrite vboxConnectGetHostname
vbox: Rewrite vboxConnectIsSecure
vbox: Rewrite vboxConnectIsEncrypted
vbox: Rewrite vboxConnectIsAlive
vbox: Rewrite vboxConnectGetMaxVcpus
vbox: Rewrite vboxConnectGetCapabilities
vbox: Rewrite vboxConnectListDomains
vbox: Rewrite vboxConnectNumOfDomains
vbox: Rewrite vboxDomainLookupById
vbox: Rewrite vboxDomainLookupByUUID
vbox: Rewrite vboxDomainUndefineFlags
vbox: Rewrite vboxDomainDefineXML
vbox: Rewrite vboxDomainCreateWithFlags
vbox: Rewrite vboxDomainCreate
vbox: Rewrite vboxDomainCreateXML
vbox: Rewrite vboxDomainLookupByName
vbox: Rewrite vboxDomainIsActive
vbox: Rewrite vboxDomainIsPersistent
vbox: Rewrite vboxDomainIsUpdated
vbox: Rewrite vboxDomainSuspend
vbox: Rewrite vboxDomainResume
vbox: Rewrite vboxDomainShutdownFlags
vbox: Rewrite vboxDomainShutdown
vbox: Rewrite vboxDomainReboot
vbox: Rewrite vboxDomainDestroyFlags
vbox: Rewrite vboxDomainDestroy
vbox: Rewrite vboxDomainGetOSType
vbox: Rewrite vboxDomainSetMemory
vbox: Rewrite vboxDomainGetInfo
vbox: Rewrite vboxDomainGetState
vbox: Rewrite vboxDomainSetVcpusFlags
vbox: Rewrite vboxDomainSetVcpus
vbox: Rewrite vboxDomainGetVcpusFlags
vbox: Rewrite vboxDomainGetMaxVcpus
vbox: Add API for vboxDomainGetXMLDesc
vbox: Rewrite vboxDomainGetXMLDesc
vbox: Rewrite vboxConnectListDefinedDomains
vbox: Rewrite vboxConnectNumOfDefinedDomains
vbox: Rewrite vboxDomainUndefine
vbox: Rewrite vboxDomainAttachDevice
vbox: Rewrite vboxDomainAttachDeviceFlags
vbox: Rewrite vboxDomainUpdateDeviceFlags
vbox: Rewrite vboxDomainDetachDevice
vbox: Rewrite vboxDomainDetachDeviceFlags
vbox: Add API for vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotGetXMLDesc
vbox: Rewrite vboxDomainSnapshotNum
vbox: Rewrite vboxDomainSnapshotListNames
vbox: Rewrite vboxSnapshotLookupByName
vbox: Rewrite vboxDomainHasCurrentSnapshot
vbox: Rewrite vboxDomainSnapshotGetParent
vbox: Rewrite vboxDomainSnapshotCurrent
vbox: Rewrite vboxDomainSnapshotIsCurrent
vbox: Rewrite vboxDomainSnapshotHasMetadata
vbox: Rewrite vboxDomainRevertToSnapshot
vbox: Rewrite vboxDomainSnapshotDelete
vbox: Rewrite vboxDomainScreenshot
vbox: Rewrite vboxConnectListAllDomains
vbox: Rewrite vboxNode functions
vbox: Add registerDomainEvent
vbox: Introducing vboxCommonDriver
po/POTFILES.in|1 +
src/Makefile.am |5 +-
src/vbox/README |7 +-
src/vbox/vbox_common.c| 7550 +
src/vbox/vbox_common.h| 306 +
src/vbox/vbox_driver.c| 40 +-
src/vbox/vbox_install_api.h | 26 +
src/vbox/vbox_tmpl.c |14557
+
src/vbox/vbox_uniformed_api.h | 551 ++
9 files changed, 13186 insertions(+), 9857 deletions(-)
create mode 100644 src/vbox/vbox_common.c
create mode 100644 src/vbox/vbox_common.h
create mode 100644 src/vbox/vbox_install_api.h
create mode 100644 src/vbox/vbox_uniformed_api.h
ACK to all the patches. I've fixed all the small nits I found. I'm
keeping the
Re: [libvirt] [PATCH v2] Perform disk config validity checking for attach-device config
On 08/07/2014 04:53 PM, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1078126 Using 'virsh attach-device --config' (or --persistent) to attach a file backed lun device will succeed; however, subsequent domain restarts will result in failure because the configuration of a file backed lun is not supported. Although allowing 'illegal configurations' is something that can be allowed, it may not be practical in this case. Generally, when attaching a device to a domain means the domain must be running. A way around this is using the --config (or --persistent) option. When an attach is done to a running domain, a temporary configuration is modified first followed by the live update. The live update will make a number of disk validity checks when building the qemu command to attach the disk. If any fail, then change is rejected. Rather than allow a potentially illegal combination, adjust the code in the configuration path to make the same checks as the running path will make with respect to disk validity checks. This way we avoid having the potential for some subsequent start/reboot to fail because an illegal combination was allowed. NB: The live path still checks the configuration since it is possible to just do --live guest modification... Signed-off-by: John Ferlan jfer...@redhat.com --- Changes since V1: http://www.redhat.com/archives/libvir-list/2014-August/msg00045.html Based on #virt IRC discussion this morning - move the qemu_caps checking to only occur during the live check - slightly change the name of the called routine src/qemu/qemu_command.c | 127 +++- src/qemu/qemu_command.h | 2 + src/qemu/qemu_driver.c | 2 + 3 files changed, 76 insertions(+), 55 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 033a5a8..c1791d9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3230,6 +3230,73 @@ qemuGetDriveSourceString(virStorageSourcePtr src, } +/* Perform disk definition config validity checks */ +int +qemuBuildCheckDiskConfig(virDomainDiskDefPtr disk) Just a nitpick: 'Build' seems redundant here, how about just 'qemuCheckDiskConfig'? ACK either way. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/66] vbox: Rewrite vbox domain driver
On Thu, Aug 14, 2014 at 05:50:18PM +0200, Yohan Belleguic wrote:
Le Wednesday 13 August 2014 17:18:44, Michal Privoznik a écrit :
[CC-ing Yohan BELLEGUIC and Manuel VIVES]
On 12.08.2014 17:31, Michal Privoznik wrote:
On 11.08.2014 12:06, Taowei wrote:
This series of patches rewrite the vbox's domain
driver. The driver is separated into two parts: the version
specified and the common part. The common driver use
vboxUniformedAPI to build a general driver for all vbox
versions. The vboxUniformedAPI take the responsiblity to
communicate with virtualbox. Since there are some incompatible
changes in virtualbox, vboxUniformedAPI should be aware of
these changes and provide a uniformed api for the upper layer.
The significant result of this patch is that we replace all
vir${vbox_version}Driver into one virCommonDriver. So, we will
have only one vbox driver implementation for all vbox versions
in libvirt.
PS: I have send part of my patches before:
https://www.redhat.com/archives/libvir-list/2014-July/msg00937.html
But I have to resend it beacuse I did some improvement on previous
patches:
*Remove the test case for vboxUniformedAPI, because it would raise
break strict-aliasing rules warning in some distibutions
*Merged the flag fdWatchNeedInitialize into domainEventCallbacks,
So, we use one flag to indicate whether vbox support callbacks
as well as we need to initialize variables for it.
Taowei (66):
vbox: Begin to rewrite, vboxConnectOpen
vbox: Rewrite vboxConnectClose
vbox: Rewrite vboxDomainSave
vbox: Rewrite vboxConnectGetVersion
vbox: Rewrite vboxConnectGetHostname
vbox: Rewrite vboxConnectIsSecure
vbox: Rewrite vboxConnectIsEncrypted
vbox: Rewrite vboxConnectIsAlive
vbox: Rewrite vboxConnectGetMaxVcpus
vbox: Rewrite vboxConnectGetCapabilities
vbox: Rewrite vboxConnectListDomains
vbox: Rewrite vboxConnectNumOfDomains
vbox: Rewrite vboxDomainLookupById
vbox: Rewrite vboxDomainLookupByUUID
vbox: Rewrite vboxDomainUndefineFlags
vbox: Rewrite vboxDomainDefineXML
vbox: Rewrite vboxDomainCreateWithFlags
vbox: Rewrite vboxDomainCreate
vbox: Rewrite vboxDomainCreateXML
vbox: Rewrite vboxDomainLookupByName
vbox: Rewrite vboxDomainIsActive
vbox: Rewrite vboxDomainIsPersistent
vbox: Rewrite vboxDomainIsUpdated
vbox: Rewrite vboxDomainSuspend
vbox: Rewrite vboxDomainResume
vbox: Rewrite vboxDomainShutdownFlags
vbox: Rewrite vboxDomainShutdown
vbox: Rewrite vboxDomainReboot
vbox: Rewrite vboxDomainDestroyFlags
vbox: Rewrite vboxDomainDestroy
vbox: Rewrite vboxDomainGetOSType
vbox: Rewrite vboxDomainSetMemory
vbox: Rewrite vboxDomainGetInfo
vbox: Rewrite vboxDomainGetState
vbox: Rewrite vboxDomainSetVcpusFlags
vbox: Rewrite vboxDomainSetVcpus
vbox: Rewrite vboxDomainGetVcpusFlags
vbox: Rewrite vboxDomainGetMaxVcpus
vbox: Add API for vboxDomainGetXMLDesc
vbox: Rewrite vboxDomainGetXMLDesc
vbox: Rewrite vboxConnectListDefinedDomains
vbox: Rewrite vboxConnectNumOfDefinedDomains
vbox: Rewrite vboxDomainUndefine
vbox: Rewrite vboxDomainAttachDevice
vbox: Rewrite vboxDomainAttachDeviceFlags
vbox: Rewrite vboxDomainUpdateDeviceFlags
vbox: Rewrite vboxDomainDetachDevice
vbox: Rewrite vboxDomainDetachDeviceFlags
vbox: Add API for vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotGetXMLDesc
vbox: Rewrite vboxDomainSnapshotNum
vbox: Rewrite vboxDomainSnapshotListNames
vbox: Rewrite vboxSnapshotLookupByName
vbox: Rewrite vboxDomainHasCurrentSnapshot
vbox: Rewrite vboxDomainSnapshotGetParent
vbox: Rewrite vboxDomainSnapshotCurrent
vbox: Rewrite vboxDomainSnapshotIsCurrent
vbox: Rewrite vboxDomainSnapshotHasMetadata
vbox: Rewrite vboxDomainRevertToSnapshot
vbox: Rewrite vboxDomainSnapshotDelete
vbox: Rewrite vboxDomainScreenshot
vbox: Rewrite vboxConnectListAllDomains
vbox: Rewrite vboxNode functions
vbox: Add registerDomainEvent
vbox: Introducing vboxCommonDriver
po/POTFILES.in|1 +
src/Makefile.am |5 +-
src/vbox/README |7 +-
src/vbox/vbox_common.c| 7550 +
src/vbox/vbox_common.h| 306 +
src/vbox/vbox_driver.c| 40 +-
src/vbox/vbox_install_api.h | 26 +
src/vbox/vbox_tmpl.c |14557
+
src/vbox/vbox_uniformed_api.h | 551 ++
9 files changed, 13186 insertions(+), 9857 deletions(-)
create mode 100644 src/vbox/vbox_common.c
create mode 100644 src/vbox/vbox_common.h
Re: [libvirt] [PATCH] cmdMigrate: move vshConnect before vshWatchJob
On 08/08/2014 10:44 AM, Chunyan Liu wrote:
A possible fix to issue:
http://www.redhat.com/archives/libvir-list/2014-August/thread.html#00227
While doing migration on KVM host, found problem sometimes:
VM is already running on the target host and disappears from source
host, but 'virsh migrate' command line hangs, cannot exit normally.
If pressing ENTER key, it will exit.
The code hangs at tools/virsh-domain.c: cmdMigrate
-vshWatchJob-poll():
poll() is trying to select pipe_fd, which is used to receive message
from doMigrate thread. In debugging, found that doMigrate finishes
and at the end it does call safewrite() to write the retval ('0' or
'1') to pipe_fd, and the write is completed. But cmdMigrate poll()
cannot get the event. If pressing ENTER key, poll() can get the
event and select pipe_fd, then command line can exit.
In current code, authentication thread which is called by vshConnect
will use stdin, and at the same time, in cmdMigrate main process,
poll() is listening to stdin, that probably affect poll() to get
pipe_fd event. Better to move authentication before vshWatchJob. With
this change, above problem does not exist.
Signed-off-by: Chunyan Liu cy...@suse.com
---
tools/virsh-domain.c | 26 --
tools/virsh.h| 1 +
2 files changed, 21 insertions(+), 6 deletions(-)
ACK
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f7193cb..2562326 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8965,6 +8965,7 @@ doMigrate(void *opaque)
virTypedParameterPtr params = NULL;
int nparams = 0;
int maxparams = 0;
+virConnectPtr dconn = data-dconn;
sigemptyset(sigmask);
sigaddset(sigmask, SIGINT);
@@ -9079,18 +9080,12 @@ doMigrate(void *opaque)
ret = '0';
} else {
I'd rather rewrite this condition as:
if (dconn) {
/* Traditional live migration */
} else {
/* peer2peer or direct */
}
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] bhyve: add volumes support
Update bhyveBuildDiskArgStr to support volumes:
- Make virBhyveProcessBuildBhyveCmd take virConnectPtr as the
first argument instead of bhyveConnPtr as virConnectPtr is
needed for virDomainTranslateDiskSourcePool,
- Add virDomainTranslateDiskSourcePool call to
virBhyveProcessBuildBhyveCmd,
- Allow disks of type VIR_STORAGE_TYPE_VOLUME
---
src/bhyve/bhyve_command.c | 8 ++--
src/bhyve/bhyve_command.h | 5 +++--
src/bhyve/bhyve_driver.c | 2 +-
src/bhyve/bhyve_process.c | 2 +-
tests/bhyvexml2argvtest.c | 5 -
5 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index e2940e8..e9fe1b7 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -184,7 +184,8 @@ bhyveBuildDiskArgStr(const virDomainDef *def
ATTRIBUTE_UNUSED,
return -1;
}
-if (virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_FILE) {
+if ((virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_FILE)
+(virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_VOLUME)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(unsupported disk type));
return -1;
@@ -209,7 +210,7 @@ bhyveBuildDiskArgStr(const virDomainDef *def
ATTRIBUTE_UNUSED,
}
virCommandPtr
-virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED,
+virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
virDomainDefPtr def, bool dryRun)
{
/*
@@ -263,6 +264,9 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver
ATTRIBUTE_UNUSED,
for (i = 0; i def-ndisks; i++) {
virDomainDiskDefPtr disk = def-disks[i];
+if (virDomainTranslateDiskSourcePool(conn, disk) 0)
+goto error;
+
if (bhyveBuildDiskArgStr(def, disk, cmd) 0)
goto error;
}
diff --git a/src/bhyve/bhyve_command.h b/src/bhyve/bhyve_command.h
index 31de97a..5b0045c 100644
--- a/src/bhyve/bhyve_command.h
+++ b/src/bhyve/bhyve_command.h
@@ -29,8 +29,9 @@
# define BHYVE_CONFIG_FORMAT_ARGV bhyve-argv
-virCommandPtr virBhyveProcessBuildBhyveCmd(bhyveConnPtr,
- virDomainDefPtr def, bool dryRun);
+virCommandPtr virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
+ virDomainDefPtr def,
+ bool dryRun);
virCommandPtr
virBhyveProcessBuildDestroyCmd(bhyveConnPtr driver,
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index eb8f9af..dcaf847 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -692,7 +692,7 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn,
if (!(loadcmd = virBhyveProcessBuildLoadCmd(privconn, def)))
goto cleanup;
-if (!(cmd = virBhyveProcessBuildBhyveCmd(privconn, def, true)))
+if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, def, true)))
goto cleanup;
virBufferAdd(buf, virCommandToString(loadcmd), -1);
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 168202e..03c72e1 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -137,7 +137,7 @@ virBhyveProcessStart(virConnectPtr conn,
goto cleanup;
/* Call bhyve to start the VM */
-if (!(cmd = virBhyveProcessBuildBhyveCmd(driver,
+if (!(cmd = virBhyveProcessBuildBhyveCmd(conn,
vm-def,
false)))
goto cleanup;
diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index 408c73a..b9be378 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -23,8 +23,11 @@ static int testCompareXMLToArgvFiles(const char *xml,
virDomainDefPtr vmdef = NULL;
virDomainObj vm;
virCommandPtr cmd = NULL;
+virConnectPtr conn;
int ret = -1;
+if (!(conn = virGetConnect()))
+goto out;
if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt,
1 VIR_DOMAIN_VIRT_BHYVE,
@@ -33,7 +36,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
vm.def = vmdef;
-if (!(cmd = virBhyveProcessBuildBhyveCmd(driver, vmdef, false)))
+if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false)))
goto out;
if (!(actualargv = virCommandToString(cmd)))
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] conf: make disk source pool translation generic
Currently, qemu driver uses qemuTranslateDiskSourcePool()
to translate disk volume information. This function is
general enough and could be used for other drivers as well,
so move it to conf/domain_conf.c along with its helpers.
- qemuTranslateDiskSourcePool: move to conf/domain_conf.c
and rename to virDomainTranslateDiskSourcePool,
- qemuAddISCSIPoolSourceHost: move to conf/domain_conf.c
and rename to virDomainAddISCSIPoolSourceHost,
- qemuTranslateDiskSourcePoolAuth: move to conf/domain_conf.c
and rename to virDomainTranslateDiskSourcePoolAuth,
- Expose virDomainTranslateDiskSourcePool through
libvirt_private.syms,
- Update users of virDomainTranslateDiskSourcePool to use a
new name.
---
src/conf/domain_conf.c | 245 +++
src/conf/domain_conf.h | 3 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_conf.c | 243 --
src/qemu/qemu_conf.h | 3 -
src/qemu/qemu_driver.c | 6 +-
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_process.c | 4 +-
tests/qemuxml2argvtest.c | 2 +-
9 files changed, 256 insertions(+), 253 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5c762fa..d5f80e0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20045,6 +20045,251 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr
def)
}
+static int
+virDomainAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
+virStoragePoolDefPtr pooldef)
+{
+int ret = -1;
+char **tokens = NULL;
+
+/* Only support one host */
+if (pooldef-source.nhost != 1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Expected exactly 1 host for the storage pool));
+goto cleanup;
+}
+
+/* iscsi pool only supports one host */
+def-src-nhosts = 1;
+
+if (VIR_ALLOC_N(def-src-hosts, def-src-nhosts) 0)
+goto cleanup;
+
+if (VIR_STRDUP(def-src-hosts[0].name, pooldef-source.hosts[0].name) 0)
+goto cleanup;
+
+if (virAsprintf(def-src-hosts[0].port, %d,
+pooldef-source.hosts[0].port ?
+pooldef-source.hosts[0].port :
+3260) 0)
+goto cleanup;
+
+/* iscsi volume has name like unit:0:0:1 */
+if (!(tokens = virStringSplit(def-src-srcpool-volume, :, 0)))
+goto cleanup;
+
+if (virStringListLength(tokens) != 4) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unexpected iscsi volume name '%s'),
+ def-src-srcpool-volume);
+goto cleanup;
+}
+
+/* iscsi pool has only one source device path */
+if (virAsprintf(def-src-path, %s/%s,
+pooldef-source.devices[0].path,
+tokens[3]) 0)
+goto cleanup;
+
+/* Storage pool have not supported these 2 attributes yet,
+ * use the defaults.
+ */
+def-src-hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
+def-src-hosts[0].socket = NULL;
+
+def-src-protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
+
+ret = 0;
+
+ cleanup:
+virStringFreeList(tokens);
+return ret;
+}
+
+
+static int
+virDomainTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def,
+ virStoragePoolSourcePtr source)
+{
+int ret = -1;
+
+/* Only necessary when authentication set */
+if (!source-auth) {
+ret = 0;
+goto cleanup;
+}
+def-src-auth = virStorageAuthDefCopy(source-auth);
+if (!def-src-auth)
+goto cleanup;
+ret = 0;
+
+ cleanup:
+return ret;
+}
+
+
+int
+virDomainTranslateDiskSourcePool(virConnectPtr conn,
+ virDomainDiskDefPtr def)
+{
+virStoragePoolDefPtr pooldef = NULL;
+virStoragePoolPtr pool = NULL;
+virStorageVolPtr vol = NULL;
+char *poolxml = NULL;
+virStorageVolInfo info;
+int ret = -1;
+virErrorPtr savedError = NULL;
+
+if (def-src-type != VIR_STORAGE_TYPE_VOLUME)
+return 0;
+
+if (!def-src-srcpool)
+return 0;
+
+if (!(pool = virStoragePoolLookupByName(conn, def-src-srcpool-pool)))
+return -1;
+
+if (virStoragePoolIsActive(pool) != 1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(storage pool '%s' containing volume '%s'
+ is not active),
+ def-src-srcpool-pool, def-src-srcpool-volume);
+goto cleanup;
+}
+
+if (!(vol = virStorageVolLookupByName(pool, def-src-srcpool-volume)))
+goto cleanup;
+
+if (virStorageVolGetInfo(vol, info) 0)
+goto cleanup;
+
+if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0)))
+goto cleanup;
+
+if (!(pooldef = virStoragePoolDefParseString(poolxml)))
+goto cleanup;
+
+def-src-srcpool-pooltype = pooldef-type;
+def-src-srcpool-voltype = info.type;
+
+if
[libvirt] [PATCH] network: fix crash when starting a network with no pf element
Martin Kletzander pointed out in email that my commit 2a193f64
introduced a crash in networkCreateInterfacePool() during startup of
any network that doesn't have a pf subelement of its forward
element. He also supplied a patch.
http://www.redhat.com/archives/libvir-list/2014-August/msg00655.html
I expanded on that patch by
cleaning uyp now-extraneous checks in the callers of
networkCreateInterfacePool().
Fortunately the offending patch hasn't been in any release, and hasn't
been (to my knowledge) backported to any other branch.
---
src/network/bridge_driver.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1ba4c3d..fc4c73d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2196,6 +2196,9 @@ networkCreateInterfacePool(virNetworkDefPtr netdef)
int ret = -1;
size_t i;
+if (!netdef-forward.npfs || netdef-forward.nifs 0)
+ return 0;
+
if ((virNetDevGetVirtualFunctions(netdef-forward.pfs-dev,
vfNames, virtFns, numVirtFns)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2204,7 +2207,6 @@ networkCreateInterfacePool(virNetworkDefPtr netdef)
goto cleanup;
}
-netdef-forward.nifs = 0;
if (VIR_ALLOC_N(netdef-forward.ifs, numVirtFns) 0)
goto cleanup;
@@ -3843,10 +3845,8 @@ networkAllocateActualDevice(virDomainDefPtr dom,
virDomainHostdevSubsysPCIBackendType backend;
iface-data.network.actual-type = actualType =
VIR_DOMAIN_NET_TYPE_HOSTDEV;
-if (netdef-forward.npfs 0 netdef-forward.nifs = 0
-networkCreateInterfacePool(netdef) 0) {
+if (networkCreateInterfacePool(netdef) 0)
goto error;
-}
/* pick first dev with 0 connections */
for (i = 0; i netdef-forward.nifs; i++) {
@@ -3978,10 +3978,8 @@ networkAllocateActualDevice(virDomainDefPtr dom,
} else {
/* pick an interface from the pool */
-if (netdef-forward.npfs 0 netdef-forward.nifs == 0
-networkCreateInterfacePool(netdef) 0) {
+if (networkCreateInterfacePool(netdef) 0)
goto error;
-}
/* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both
* require exclusive access to a device, so current
@@ -4149,10 +4147,9 @@ networkNotifyActualDevice(virDomainDefPtr dom,
goto success;
}
-if (netdef-forward.npfs 0 netdef-forward.nifs == 0
-networkCreateInterfacePool(netdef) 0) {
+if (networkCreateInterfacePool(netdef) 0)
goto error;
-}
+
if (netdef-forward.nifs == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(network '%s' uses a direct or hostdev mode,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V5 10/12] src/xenxs: Refactor code formating Vfb config
Kiarie Kahurani wrote:
introduce function
xenFormatXMVfb(virConfPtr conf,.);
which formats Vfb config instead
Continuing my review from yesterday...
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 171
++---
1 file changed, 97 insertions(+), 74 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 9edfbbb..4795644 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -2102,108 +2102,65 @@ xenFormatXMOS(virConfPtr conf, virDomainDefPtr def,
return 0;
}
-/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
- either 32, or 64 on a platform where long is big enough. */
-verify(MAX_VIRT_CPUS = sizeof(1UL) * CHAR_BIT);
-
-virConfPtr
-xenFormatXM(virConnectPtr conn,
-virDomainDefPtr def,
-int xendConfigVersion)
-{
-virConfPtr conf = NULL;
-int hvm = 0;
-size_t i;
-virConfValuePtr netVal = NULL;
-
-if (!(conf = virConfNew()))
-goto cleanup;
-
-if (xenFormatXMGeneralMeta(conf, def) 0)
-goto cleanup;
-
-if (xenFormatXMMem(conf, def) 0)
-goto cleanup;
-
-if (xenFormatXMCPUFeatures(conf, def, xendConfigVersion) 0)
-goto cleanup;
-
-hvm = STREQ(def-os.type, hvm);
-if (xenFormatXMOS(conf, def, xendConfigVersion) 0)
-goto cleanup;
-
-if (xenFormatXMTimeOffset(conf, def, xendConfigVersion) 0)
-goto cleanup;
-if (xenFormatXMEventActions(conf, def) 0)
-goto cleanup;
-
-if (hvm) {
-for (i = 0; i def-ninputs; i++) {
-if (def-inputs[i]-bus == VIR_DOMAIN_INPUT_BUS_USB) {
-if (xenXMConfigSetInt(conf, usb, 1) 0)
-goto cleanup;
-switch (def-inputs[i]-type) {
-case VIR_DOMAIN_INPUT_TYPE_MOUSE:
-if (xenXMConfigSetString(conf, usbdevice, mouse)
0)
-goto cleanup;
-break;
-case VIR_DOMAIN_INPUT_TYPE_TABLET:
-if (xenXMConfigSetString(conf, usbdevice,
tablet) 0)
-goto cleanup;
-break;
-case VIR_DOMAIN_INPUT_TYPE_KBD:
-if (xenXMConfigSetString(conf, usbdevice,
keyboard) 0)
-goto cleanup;
-break;
-}
-break;
-}
-}
-}
+static int
+xenFormatXMVfb(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
Previously mentioned style nit. Will fix before pushing. Otherwise ACK.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cmdMigrate: move vshConnect before vshWatchJob
Hi Jan,
Is the auto login a pre-requisite for p2p direct migration ? The
current patch mostly focus on avoiding the virsh migrate hang.
Thanks,
Shiva
On Thu, Aug 14, 2014 at 9:40 PM, Ján Tomko jto...@redhat.com wrote:
On 08/08/2014 10:44 AM, Chunyan Liu wrote:
A possible fix to issue:
http://www.redhat.com/archives/libvir-list/2014-August/thread.html#00227
While doing migration on KVM host, found problem sometimes:
VM is already running on the target host and disappears from source
host, but 'virsh migrate' command line hangs, cannot exit normally.
If pressing ENTER key, it will exit.
The code hangs at tools/virsh-domain.c: cmdMigrate
-vshWatchJob-poll():
poll() is trying to select pipe_fd, which is used to receive message
from doMigrate thread. In debugging, found that doMigrate finishes
and at the end it does call safewrite() to write the retval ('0' or
'1') to pipe_fd, and the write is completed. But cmdMigrate poll()
cannot get the event. If pressing ENTER key, poll() can get the
event and select pipe_fd, then command line can exit.
In current code, authentication thread which is called by vshConnect
will use stdin, and at the same time, in cmdMigrate main process,
poll() is listening to stdin, that probably affect poll() to get
pipe_fd event. Better to move authentication before vshWatchJob. With
this change, above problem does not exist.
Signed-off-by: Chunyan Liu cy...@suse.com
---
tools/virsh-domain.c | 26 --
tools/virsh.h| 1 +
2 files changed, 21 insertions(+), 6 deletions(-)
ACK
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f7193cb..2562326 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8965,6 +8965,7 @@ doMigrate(void *opaque)
virTypedParameterPtr params = NULL;
int nparams = 0;
int maxparams = 0;
+virConnectPtr dconn = data-dconn;
sigemptyset(sigmask);
sigaddset(sigmask, SIGINT);
@@ -9079,18 +9080,12 @@ doMigrate(void *opaque)
ret = '0';
} else {
I'd rather rewrite this condition as:
if (dconn) {
/* Traditional live migration */
} else {
/* peer2peer or direct */
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V5 11/12] src/xenxs: Refactor code formating emulated devices config
Kiarie Kahurani wrote:
introduce function
xenFormatXMEmulatedHardware(virConfPtr conf,);
You've also added xenFormatXMVif in this patch. It should be in a
separate patch IMO. Once removed, a better description of this patch is
src/xenxs: Refactor code formating peripheral device config since it
parses input devices, sound, etc.
which formats emulated hardware config instead
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 150
+
tests/xmconfigdata/test-escape-paths.cfg | 2 +-
tests/xmconfigdata/test-fullvirt-sound.cfg | 2 +-
3 files changed, 92 insertions(+), 62 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 4795644..9718c92 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -2221,6 +2221,93 @@ xenFormatXMVfb(virConfPtr conf, virDomainDefPtr def,
return 0;
}
+
+
+static int
+xenFormatXMVif(virConfPtr conf, virConnectPtr conn,
+ virDomainDefPtr def, int xendConfigVersion)
+{
+ virConfValuePtr netVal = NULL;
+ size_t i;
+ int hvm = STREQ(def-os.type, hvm);
+
+ if (VIR_ALLOC(netVal) 0)
+goto cleanup;
+netVal-type = VIR_CONF_LIST;
+netVal-list = NULL;
+
+for (i = 0; i def-nnets; i++) {
+if (xenFormatXMNet(conn, netVal, def-nets[i],
+ hvm, xendConfigVersion) 0)
+ goto cleanup;
+}
+
+if (netVal-list != NULL) {
+int ret = virConfSetValue(conf, vif, netVal);
+netVal = NULL;
+if (ret 0)
+goto cleanup;
+}
+
+VIR_FREE(netVal);
+return 0;
+
+ cleanup:
+virConfFreeValue(netVal);
+return -1;
+}
+
+
+static int
+xenFormatXMEmulatedHardware(virConfPtr conf, virDomainDefPtr def)
I've split this into xenFormatXMInputDevs and xenFormatXMSound.
+{
+size_t i;
+
+if (STREQ(def-os.type, hvm)) {
+if (def-sounds) {
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *str = NULL;
+int ret = xenFormatSxprSound(def, buf);
+str = virBufferContentAndReset(buf);
+if (ret == 0)
+ret = xenXMConfigSetString(conf, soundhw, str);
+
+VIR_FREE(str);
+if (ret 0)
+return -1;
+}
+
+for (i = 0; i def-ninputs; i++) {
+if (def-inputs[i]-bus == VIR_DOMAIN_INPUT_BUS_USB) {
+if (xenXMConfigSetInt(conf, usb, 1) 0)
+return -1;
+
+switch (def-inputs[i]-type) {
+case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+if (xenXMConfigSetString(conf, usbdevice, mouse)
0)
+return -1;
+
+break;
+case VIR_DOMAIN_INPUT_TYPE_TABLET:
+if (xenXMConfigSetString(conf, usbdevice,
tablet) 0)
+return -1;
+
+break;
+case VIR_DOMAIN_INPUT_TYPE_KBD:
+if (xenXMConfigSetString(conf, usbdevice,
keyboard) 0)
+return -1;
+
+break;
+}
+break;
+}
+}
+}
+
+return 0;
+}
+
+
/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
either 32, or 64 on a platform where long is big enough. */
verify(MAX_VIRT_CPUS = sizeof(1UL) * CHAR_BIT);
@@ -2231,9 +2318,6 @@ xenFormatXM(virConnectPtr conn,
int xendConfigVersion)
{
virConfPtr conf = NULL;
-int hvm = 0;
-size_t i;
-virConfValuePtr netVal = NULL;
if (!(conf = virConfNew()))
goto cleanup;
@@ -2247,8 +2331,6 @@ xenFormatXM(virConnectPtr conn,
if (xenFormatXMCPUFeatures(conf, def, xendConfigVersion) 0)
goto cleanup;
-hvm = STREQ(def-os.type, hvm);
-
if (xenFormatXMOS(conf, def, xendConfigVersion) 0)
goto cleanup;
@@ -2258,29 +2340,8 @@ xenFormatXM(virConnectPtr conn,
if (xenFormatXMEventActions(conf, def) 0)
goto cleanup;
-if (hvm) {
-for (i = 0; i def-ninputs; i++) {
-if (def-inputs[i]-bus == VIR_DOMAIN_INPUT_BUS_USB) {
-if (xenXMConfigSetInt(conf, usb, 1) 0)
-goto cleanup;
-switch (def-inputs[i]-type) {
-case VIR_DOMAIN_INPUT_TYPE_MOUSE:
-if (xenXMConfigSetString(conf, usbdevice, mouse)
0)
-goto cleanup;
-break;
-case VIR_DOMAIN_INPUT_TYPE_TABLET:
-if (xenXMConfigSetString(conf, usbdevice,
tablet) 0)
-goto cleanup;
-break;
-case
[libvirt] [PATCH] qemu_process: fix memleak found by coverity
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
Pushed as trivial
src/qemu/qemu_process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 13c396f..52d9052 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2323,8 +2323,10 @@ qemuProcessInitPasswords(virConnectPtr conn,
goto cleanup;
alias = vm-def-disks[i]-info.alias;
-if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0)
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0) {
+VIR_FREE(secret);
goto cleanup;
+}
ret = qemuMonitorSetDrivePassphrase(priv-mon, alias, secret);
VIR_FREE(secret);
qemuDomainObjExitMonitor(driver, vm);
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V5 00/12] xen_xm.c code refactor
Kiarie Kahurani wrote:
Kiarie Kahurani (12):
src/xenxs: Export code for reuse
[...]
src/xenxs: Refactor code formating general VM config
src/xenxs: Refactor code formating memory config
src/xenxs: Refactor code formating virtual time config
src/xenxs: Refactor code formating event actions config
src/xenxs: Refactor code formating Char devices config
src/xenxs: Refactor code formating xm disk config
src/xenxs: Refactor code formating CPU features config
src/xenxs: Refactor code formating OS config
src/xenxs: Refactor code formating Vfb config
src/xenxs: Refactor code formating emulated devices config
I've pushed 2-11 plus the additional patch for vif formating.
I planned to defer 1/12 and 12/12 until reworking the xenxs directory
https://www.redhat.com/archives/libvir-list/2014-August/msg00519.html
But I think it would be better to have them before creating
xen_common.[ch], to avoid temporarily exporting all the
xen{Parse,Format}XM* functions in xen_common.h. In the end, we'll only
need to export xen{Parse,Format}Common.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: fix crash when starting a network with no pf element
On Thu, Aug 14, 2014 at 12:40:07PM -0400, Laine Stump wrote: Martin Kletzander pointed out in email that my commit 2a193f64 introduced a crash in networkCreateInterfacePool() during startup of any network that doesn't have a pf subelement of its forward element. He also supplied a patch. http://www.redhat.com/archives/libvir-list/2014-August/msg00655.html I expanded on that patch by cleaning uyp now-extraneous checks in the callers of s/uyp/up/ networkCreateInterfacePool(). Fortunately the offending patch hasn't been in any release, and hasn't been (to my knowledge) backported to any other branch. --- src/network/bridge_driver.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1ba4c3d..fc4c73d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2196,6 +2196,9 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) int ret = -1; size_t i; +if (!netdef-forward.npfs || netdef-forward.nifs 0) I'd match these to use both !var (or var != 0), but other than that the patch is fine, ACK with that fixed. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Can comments be added to libvirt xml files?
Hi. Is it possible to preserve comments in libvirt's XML files without it failing validate-xml checks? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Suboptimal default cpu Cgroup
Hello,
by default, libvirt with KVM creates a Cgroup hierarchy in 'cpu,cpuacct'
[1], with 'shares' set to 1024 on every level. This raises two points:
1) Every VM is given an equal amount of CPU time. [2]
($CG/machine.slice/*/shares = 1024)
Which means that smaller / less loaded guests are given an advantage.
2) All VMs combined are given 1024 shares. [3]
($CG/machine.slice/shares)
This is made even worse on RHEL7, by sched_autogroup_enabled = 0, so
every other process in the system is given the same amount of CPU as
all VMs combined.
It does not seem to be possible to tune shares and get a good general
behavior, so the best solution I can see is to disable the cpu cgroup
and let users do it when needed. (Keeping all tasks in $CG/tasks.)
Do we want cgroups in the default at all?
(Is OpenStack dealing with these quirks?)
Thanks.
---
1: machine.slice/
machine-qemu\\x2d${name}.scope/
{emulator,vcpu*}/
2: To reproduce, run two guests with 1 VCPU and execute two spinners
on the first and one on the second.
The result will be 50%/50% CPU assignment between guests; 66%/33%
seems more natural, but it could still be considered as a feature.
3: Run a guest with $n VCPUs and $n spinners in it, and $n spinners in
the host
- RHEL7: 1/($n + 1)% CPU for the guest -- I'd expect 50%/50%.
- Upstream: 50%/50% between guest and host because of autogrouping;
if you run $n more spinners in the host, it will still be 50%/50%,
instead of seemingly more fair 33%/66%. (And you can run spinners
from different groups, so it would be the same as in RHEL7 then.)
And it also works the other way: if the host has $n CPUs, then
$n/2 tasks in the host suffice to minimize VMs' performance,
regardless of the amount of running VCPUs.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Suboptimal default cpu Cgroup
2014-08-14 13:55-0400, Andrew Theurer: - Original Message - From: Radim Krčmář rkrc...@redhat.com To: libvir-list@redhat.com Cc: Daniel P. Berrange berra...@redhat.com, Andrew Theurer atheu...@redhat.com Sent: Thursday, August 14, 2014 9:25:05 AM Subject: Suboptimal default cpu Cgroup Hello, by default, libvirt with KVM creates a Cgroup hierarchy in 'cpu,cpuacct' [1], with 'shares' set to 1024 on every level. This raises two points: 1) Every VM is given an equal amount of CPU time. [2] ($CG/machine.slice/*/shares = 1024) Which means that smaller / less loaded guests are given an advantage. 2) All VMs combined are given 1024 shares. [3] ($CG/machine.slice/shares) This is made even worse on RHEL7, by sched_autogroup_enabled = 0, so every other process in the system is given the same amount of CPU as all VMs combined. It does not seem to be possible to tune shares and get a good general behavior, so the best solution I can see is to disable the cpu cgroup and let users do it when needed. (Keeping all tasks in $CG/tasks.) Could we have each VM's shares be nr_vcpu * 1024, and the share for $CG/machine.slice be sum of all VM's share? That would be unfair in a different way ... some examples: VM's shares = nr_vcpu * 1024: - 1 and 10 VCPU guests both running only one task in overcommit, larger guest gets 10 times more CPU. (Feature?) $CG/machine.slice = sum (VM's shares): - 'shares' are bound by 262144 right now, so it wouldn't scale beyond one large guest. (Not a big problem, but has ugly solutions.) - Default system tasks still have 1024, so their share would get unfairly small if we had some idle guests as well. 10 CPU machine with 10*10 VCPU guests, only one of which is actively running: A non-vm task would get just ~1% of the CPU, not ~10%, like we would expect with 11 running tasks. And it would be even worse with autogrouping. --- [...] 2: To reproduce, run two guests with 1 VCPU and execute two spinners on the first and one on the second. The result will be 50%/50% CPU assignment between guests; 66%/33% seems more natural, but it could still be considered as a feature. (Please note a mistake here: the host is implied to have 1-2 CPUs. It would have been better to use nr_cpus as well ...) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Suboptimal default cpu Cgroup
- Original Message -
From: Radim Krčmář rkrc...@redhat.com
To: libvir-list@redhat.com
Cc: Daniel P. Berrange berra...@redhat.com, Andrew Theurer
atheu...@redhat.com
Sent: Thursday, August 14, 2014 9:25:05 AM
Subject: Suboptimal default cpu Cgroup
Hello,
by default, libvirt with KVM creates a Cgroup hierarchy in 'cpu,cpuacct'
[1], with 'shares' set to 1024 on every level. This raises two points:
1) Every VM is given an equal amount of CPU time. [2]
($CG/machine.slice/*/shares = 1024)
Which means that smaller / less loaded guests are given an advantage.
2) All VMs combined are given 1024 shares. [3]
($CG/machine.slice/shares)
This is made even worse on RHEL7, by sched_autogroup_enabled = 0, so
every other process in the system is given the same amount of CPU as
all VMs combined.
It does not seem to be possible to tune shares and get a good general
behavior, so the best solution I can see is to disable the cpu cgroup
and let users do it when needed. (Keeping all tasks in $CG/tasks.)
Could we have each VM's shares be nr_vcpu * 1024, and the share for
$CG/machine.slice be sum of all VM's share?
Do we want cgroups in the default at all?
(Is OpenStack dealing with these quirks?)
Thanks.
---
1: machine.slice/
machine-qemu\\x2d${name}.scope/
{emulator,vcpu*}/
2: To reproduce, run two guests with 1 VCPU and execute two spinners
on the first and one on the second.
The result will be 50%/50% CPU assignment between guests; 66%/33%
seems more natural, but it could still be considered as a feature.
3: Run a guest with $n VCPUs and $n spinners in it, and $n spinners in
the host
- RHEL7: 1/($n + 1)% CPU for the guest -- I'd expect 50%/50%.
- Upstream: 50%/50% between guest and host because of autogrouping;
if you run $n more spinners in the host, it will still be 50%/50%,
instead of seemingly more fair 33%/66%. (And you can run spinners
from different groups, so it would be the same as in RHEL7 then.)
And it also works the other way: if the host has $n CPUs, then
$n/2 tasks in the host suffice to minimize VMs' performance,
regardless of the amount of running VCPUs.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: force configure failed when perl is missing
On 08/14/2014 05:33 PM, Michal Privoznik wrote:
On 14.08.2014 05:37, Jincheng Miao wrote:
Perl is necessary to our build processing, it will invoke a lot of
generating script, like: gendispatch.pl. If perl is missing, it's
ok for build from git checkout, because autogen.sh will tell you.
But for compiling from a release tarball, configure will just record
a missing message, and continue, then build failed, like:
https://www.redhat.com/archives/libvirt-users/2014-August/msg00050.html
So need to enhance configure script to handle this negative case.
Reported-by: Hongbin Lu hong...@savinetwork.ca
Signed-off-by: Jincheng Miao jm...@redhat.com
---
configure.ac |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac
index 081f298..af3fe28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2173,6 +2173,9 @@ AM_CONDITIONAL([WITH_HYPERV], [test
$with_hyperv = yes])
dnl Allow perl/python overrides
AC_PATH_PROGS([PYTHON], [python2 python])
AC_PATH_PROG([PERL], [perl])
+if test -z $PERL; then
+ AC_MSG_ERROR([Failed to find perl.])
+fi
AC_ARG_WITH([test-suite],
[AS_HELP_STRING([--with-test-suite],
I'm inclined to ACK this. We currently have some files that can be
built from git and are contained in the release so we cut off the set
of required tools to build the libvirt. However, with so widely
accessible tool as perl (which is almost everywhere) we don't need to
do that. Moreover, there are some Makefile targets which have runtime
dependency on perl, e.g. bracket-spacing-check syntax-check rule.
And with this change I think we need this one too:
Yes, the BuildRequires should also be added in libvirt.spec.in.
Thanks for the review both of you.
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 29da071..f491de7 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -446,6 +446,7 @@ BuildRequires: gettext-devel
BuildRequires: libtool
BuildRequires: /usr/bin/pod2man
%endif
+BuildRequires: perl
BuildRequires: python
%if %{with_systemd}
BuildRequires: systemd-units
Again, I'm not expecting to see any RPM based distribution without
perl (esp. if rpm-build package requires perl itself). But we should
have it for completeness.
So I'm squashing it in and pushing.
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Fix build warning on libvirt-python
Ping. [python PATCH]
On 2014/8/8 17:55, Wang Rui wrote:
From: Mo Yuxiang moyuxi...@huawei.com
On compiling libvirt-python, we get such a warning:
libvirt-qemu-override.c: In function
‘libvirt_qemu_virConnectDomainQemuMonitorEventRegister’:
libvirt-qemu-override.c:304: warning: suggest explicit braces to avoid
ambiguous ‘else’
Py_DECREF is a macro. The solution is to add brackets.
Signed-off-by: Mo Yuxiang moyuxi...@huawei.com
---
libvirt-qemu-override.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libvirt-qemu-override.c b/libvirt-qemu-override.c
index 05ead30..8be3755 100644
--- a/libvirt-qemu-override.c
+++ b/libvirt-qemu-override.c
@@ -301,8 +301,9 @@
libvirt_qemu_virConnectDomainQemuMonitorEventRegister(PyObject *self
ATTRIBUTE_U
flags);
LIBVIRT_END_ALLOW_THREADS;
-if (ret 0)
+if (ret 0) {
Py_DECREF(pyobj_cbData);
+}
py_retval = libvirt_intWrap(ret);
return py_retval;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix
On 14/08/14 20:14, Ján Tomko wrote: On 08/14/2014 11:52 AM, Michal Privoznik wrote: On 14.08.2014 10:41, Ján Tomko wrote: Also add qemuDomainChangeGraphicsPasswords, qemuProcessVerifyGuestCPU and qemuProcessInitPCIAddresses. Replace tabs by spaces. --- I'll do some testing on the patch and push it later with this squashed in if it works well. src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_hotplug.c | 12 src/qemu/qemu_hotplug.h | 3 ++- src/qemu/qemu_process.c | 33 - 4 files changed, 31 insertions(+), 19 deletions(-) ACK to the original with this squashed in. Now pushed. Jan Jan, Michal, Thanks for your help :-) (I'll be more careful with formatting in future.) Cheers, Sam. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can comments be added to libvirt xml files?
On 08/15/2014 12:36 AM, ban...@openmailbox.org wrote: Hi. Is it possible to preserve comments in libvirt's XML files without it failing validate-xml checks? Any comments you add to the XML will be lost when libvirt parses and formats it again, only the elements libvirt knows will be preserved. For domains, we have a few elements where you can store your own data: http://libvirt.org/formatdomain.html#elementsMetadata For secrets and snapshots, only 'description' is available. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
