[libvirt] Fwd: Re: [Qemu-devel] [edk2 PATCH] OvmfPkg: split the variable store to a separate file
by Eric Blake
[Forwarding with Laszlo's permission - if ever you wonder whether your
work on libvirt makes a difference, it does!]
-------- Original Message --------
Message-ID: <528FCF54.6060203(a)redhat.com>
Date: Fri, 22 Nov 2013 22:40:36 +0100
From: Laszlo Ersek <lersek(a)redhat.com>
Subject: Re: [Qemu-devel] [edk2 PATCH] OvmfPkg: split the variable store
to a separate file
On 11/22/13 21:51, Jordan Justen wrote:
> On Fri, Nov 22, 2013 at 10:43 AM, Laszlo Ersek <lersek(a)redhat.com> wrote:
>> OTOH building all three FDs per default might be confusing for
>> end-users. We know for a fact that they usually don't read the README
>> (or not thoroughly enough). If we only give them one output file per
>> default, that's less potential for confusion.
>
> If it will just add confusion, then perhaps it is something best left
> out of the README. And at that point, since splitting it is so easy,
> the value of having it in OVMF is debatable.
>
>> But I can certainly post a version where all three files are built per
>> default, if that's what you prefer (and aren't opposed to the idea in
>> general).
>
> I'm not opposed to it, but I would like to wait to see what QEMU does.
OK. Let's see where the qemu patch goes (I'll have to fix the comment
typo that Eric found), and if it is accepted (maybe with modifications),
we can return to the OVMF patch.
> Many of these scenarios were discussed in the past on qemu-devel, but
> a single -pflash was the only thing that stuck.
But (thanks to you) we now have a flash driver in OVMF that works *in
practice*. The discussion about multiple -pflash flags is not academic
any longer. (Hm, sorry, that's maybe too strong -- I can't really say if
the discussion used to be academic before. But it cannot have been this
practical.)
We also know that the libvirt developers prefer the split file. Plus:
> This has made me just
> focus on making the single file pflash work. You also can't deny that
> the QEMU command line gets ugly real fast.
We're in violent agreement on that -- which emphasizes the need for good
libvirt support all the more.
Seriously, I refuse to work with the qemu command line day to day. I
have an elaborate wrapper script that I insert between qemu and libvirt
(specified in the <emulator> libvirt domain XML) that post-processes the
arguments composed by libvirt. I need this to test out features that
libvirt doesn't yet support.
For example, I can set various filenames in the <loader> element -- it
specifies the argument to the -bios option. In the script I check the
argument against various patterns, and I can turn it into:
- -bios "$ARG"
- -pflash "$ARG"
- -pflash "$ARG" -pflash "$(manipulate "$ARG")"
The script handles CSM vs. pure-UEFI for Windows 2008, it can honor
upstream vs. RHEL qemu differences, it can turn off the iPXE virtio-net
driver. In the single -pflash option case, it does refresh the code part
in the VM's copy of OVMF (with dd) from my most recent build. It sets a
name-dependent output file for the debug port. Etc.
Maintaining this script over months has paid off orders of magnitude
better than working with the raw qemu command line would have. (I can
compare: on my (occasionally used) AMD desktop that runs Debian I have
not bothered to install libvirt, and each time I need to modify the
forty line qemu command that I keep in a script file, I cringe. And I
can't bring myself to install a second guest.)
The convenience/efficiency libvirt gives me is critical. Changing boot
order, adding or removing devices, starting & stopping -- these are very
fast in virt-manager. Editing the configuration with virsh is nice (and
it has some level of automatic verification when you save and exit).
Comparing guest configs against each other (using the XMLs) is very
convenient. And so on. I depend on these every day, but I need to modify
the wrapper script only occasionally.
Libvirt still allows me to send custom monitor commands, and I can set
it to log all of its *own* commands that it sends to the monitor or the
guest agent.
I've always wondered how other developers (for example, you :)) can
exist without libvirt at all!
That is why "keep the qemu command line simple" (== approx. "two -pflash
options are inconvenient") is no concern of mine. That ship has sailed.
Thanks,
Laszlo
11 years, 1 month
[libvirt] [PATCH] storage: allow interleave in volume XML
by Eric Blake
The RNG grammar did not allow arbitrary interleaving, which makes
it harder than necessary to create a new volume from handwritten XML.
(Compare also to commit caf516db for pools).
* docs/schemas/storagevol.rng: Support interleaving.
* tests/storagevolxml2xmlin/vol-file-backing.xml: Test it.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
reviewer's note: Look after the '-- ' trailer for a
smaller version of the patch generated with 'git diff -w';
that version won't apply, but makes it easier to see
the real changes rather than getting lost in indentation.
docs/schemas/storagevol.rng | 92 ++++++++++++++------------
tests/storagevolxml2xmlin/vol-file-backing.xml | 15 +++--
2 files changed, 57 insertions(+), 50 deletions(-)
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index ff7a952..e79bc35 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -12,8 +12,8 @@
<define name='vol'>
- <interleave>
- <element name='volume'>
+ <element name='volume'>
+ <interleave>
<element name='name'>
<ref name='volName'/>
</element>
@@ -30,8 +30,8 @@
<optional>
<ref name='backingStore'/>
</optional>
- </element>
- </interleave>
+ </interleave>
+ </element>
</define>
<define name='sizing'>
@@ -52,20 +52,22 @@
<define name='permissions'>
<optional>
<element name='permissions'>
- <element name='mode'>
- <ref name='octalMode'/>
- </element>
- <element name='owner'>
- <ref name='unsignedInt'/>
- </element>
- <element name='group'>
- <ref name='unsignedInt'/>
- </element>
- <optional>
- <element name='label'>
- <text/>
- </element>
- </optional>
+ <interleave>
+ <element name='mode'>
+ <ref name='octalMode'/>
+ </element>
+ <element name='owner'>
+ <ref name='unsignedInt'/>
+ </element>
+ <element name='group'>
+ <ref name='unsignedInt'/>
+ </element>
+ <optional>
+ <element name='label'>
+ <text/>
+ </element>
+ </optional>
+ </interleave>
</element>
</optional>
</define>
@@ -107,36 +109,40 @@
<define name='target'>
<element name='target'>
- <optional>
- <element name='path'>
- <choice>
- <data type='anyURI'/>
- <ref name='absFilePath'/>
- </choice>
- </element>
- </optional>
- <ref name='format'/>
- <ref name='permissions'/>
- <ref name='timestamps'/>
- <optional>
- <ref name='encryption'/>
- </optional>
- <optional>
- <ref name='compat'/>
- </optional>
- <optional>
- <ref name='fileFormatFeatures'/>
- </optional>
+ <interleave>
+ <optional>
+ <element name='path'>
+ <choice>
+ <data type='anyURI'/>
+ <ref name='absFilePath'/>
+ </choice>
+ </element>
+ </optional>
+ <ref name='format'/>
+ <ref name='permissions'/>
+ <ref name='timestamps'/>
+ <optional>
+ <ref name='encryption'/>
+ </optional>
+ <optional>
+ <ref name='compat'/>
+ </optional>
+ <optional>
+ <ref name='fileFormatFeatures'/>
+ </optional>
+ </interleave>
</element>
</define>
<define name='backingStore'>
<element name='backingStore'>
- <element name='path'>
- <ref name='absFilePath'/>
- </element>
- <ref name='format'/>
- <ref name='permissions'/>
+ <interleave>
+ <element name='path'>
+ <ref name='absFilePath'/>
+ </element>
+ <ref name='format'/>
+ <ref name='permissions'/>
+ </interleave>
</element>
</define>
diff --git a/tests/storagevolxml2xmlin/vol-file-backing.xml b/tests/storagevolxml2xmlin/vol-file-backing.xml
index 73e7f28..8610ea4 100644
--- a/tests/storagevolxml2xmlin/vol-file-backing.xml
+++ b/tests/storagevolxml2xmlin/vol-file-backing.xml
@@ -1,25 +1,26 @@
<volume>
- <name>sparse.img</name>
+ <!-- lines scrambled to test interleaves -->
<key>/var/lib/libvirt/images/sparse.img</key>
- <source/>
<capacity unit='GB'>10</capacity>
- <allocation unit='MiB'>0</allocation>
+ <source/>
<target>
- <path>/var/lib/libvirt/images/sparse.img</path>
<permissions>
- <mode>0</mode>
<owner>0744</owner>
+ <mode>0</mode>
<group>0</group>
</permissions>
+ <path>/var/lib/libvirt/images/sparse.img</path>
</target>
+ <allocation unit='MiB'>0</allocation>
<backingStore>
- <path>/var/lib/virt/images/master.img</path>
<format type='vmdk'/>
+ <path>/var/lib/virt/images/master.img</path>
<permissions>
<mode>0744</mode>
+ <label>virt_image_t</label>
<owner>1</owner>
<group>1</group>
- <label>virt_image_t</label>
</permissions>
</backingStore>
+ <name>sparse.img</name>
</volume>
--
1.8.3.1
As promised above, the docs/schemas change in an easier format:
diff --git c/docs/schemas/storagevol.rng w/docs/schemas/storagevol.rng
index ff7a952..e79bc35 100644
--- c/docs/schemas/storagevol.rng
+++ w/docs/schemas/storagevol.rng
@@ -12,8 +12,8 @@
<define name='vol'>
- <interleave>
<element name='volume'>
+ <interleave>
<element name='name'>
<ref name='volName'/>
</element>
@@ -30,8 +30,8 @@
<optional>
<ref name='backingStore'/>
</optional>
- </element>
</interleave>
+ </element>
</define>
<define name='sizing'>
@@ -52,6 +52,7 @@
<define name='permissions'>
<optional>
<element name='permissions'>
+ <interleave>
<element name='mode'>
<ref name='octalMode'/>
</element>
@@ -66,6 +67,7 @@
<text/>
</element>
</optional>
+ </interleave>
</element>
</optional>
</define>
@@ -107,6 +109,7 @@
<define name='target'>
<element name='target'>
+ <interleave>
<optional>
<element name='path'>
<choice>
@@ -127,16 +130,19 @@
<optional>
<ref name='fileFormatFeatures'/>
</optional>
+ </interleave>
</element>
</define>
<define name='backingStore'>
<element name='backingStore'>
+ <interleave>
<element name='path'>
<ref name='absFilePath'/>
</element>
<ref name='format'/>
<ref name='permissions'/>
+ </interleave>
</element>
</define>
11 years, 1 month
[libvirt] [PATCH] virsh domxml-from-native to treat SCSI as the bus type for pseries by default
by Shivaprasad G Bhat
From: Shivaprasad G Bhat <shivaprasadbhat(a)gmail.com>
The bus type IDE being enum Zero, the bus type on pseries system appears as IDE for all the -hda/-cdrom and for disk drives with if="none" type. Pseries platform needs this to appear as SCSI instead of IDE. The ide being not supported, the explicit requests for ide devices will return an error.
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 66 +++++++++++++++++++-
tests/qemuargv2xmltest.c | 1
.../qemuxml2argv-pseries-disk.args | 5 ++
.../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 40 ++++++++++++
4 files changed, 108 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8dc7e43..21b5108 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10032,6 +10032,7 @@ error:
static virDomainDiskDefPtr
qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
const char *val,
+ virDomainDefPtr dom,
int nvirtiodisk,
bool old_style_ceph_args)
{
@@ -10055,7 +10056,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
if (VIR_ALLOC(def) < 0)
goto cleanup;
- def->bus = VIR_DOMAIN_DISK_BUS_IDE;
+ if (((dom->os.arch == VIR_ARCH_PPC64) &&
+ dom->os.machine && STREQ(dom->os.machine, "pseries")))
+ def->bus = VIR_DOMAIN_DISK_BUS_SCSI;
+ else
+ def->bus = VIR_DOMAIN_DISK_BUS_IDE;
def->device = VIR_DOMAIN_DISK_DEVICE_DISK;
def->type = VIR_DOMAIN_DISK_TYPE_FILE;
@@ -10140,9 +10145,15 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
def->type = VIR_DOMAIN_DISK_TYPE_FILE;
}
} else if (STREQ(keywords[i], "if")) {
- if (STREQ(values[i], "ide"))
+ if (STREQ(values[i], "ide")) {
def->bus = VIR_DOMAIN_DISK_BUS_IDE;
- else if (STREQ(values[i], "scsi"))
+ if (((dom->os.arch == VIR_ARCH_PPC64) &&
+ dom->os.machine && STREQ(dom->os.machine, "pseries"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("pseries systems do not support ide devices '%s'"), val);
+ goto error;
+ }
+ } else if (STREQ(values[i], "scsi"))
def->bus = VIR_DOMAIN_DISK_BUS_SCSI;
else if (STREQ(values[i], "virtio"))
def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO;
@@ -11368,6 +11379,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
if (STREQ(arg, "-cdrom")) {
disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
+ if (((def->os.arch == VIR_ARCH_PPC64) &&
+ def->os.machine && STREQ(def->os.machine, "pseries")))
+ disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
if (VIR_STRDUP(disk->dst, "hdc") < 0)
goto error;
disk->readonly = true;
@@ -11381,6 +11395,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
else
disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
+ if (((def->os.arch == VIR_ARCH_PPC64) &&
+ def->os.machine && STREQ(def->os.machine, "pseries")))
+ disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
}
if (VIR_STRDUP(disk->dst, arg + 1) < 0)
goto error;
@@ -11672,7 +11689,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
}
} else if (STREQ(arg, "-drive")) {
WANT_VALUE();
- if (!(disk = qemuParseCommandLineDisk(xmlopt, val,
+ if (!(disk = qemuParseCommandLineDisk(xmlopt, val, def,
nvirtiodisk,
ceph_args != NULL)))
goto error;
@@ -11858,6 +11875,47 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
}
} else if (STREQ(arg, "-S")) {
/* ignore, always added by libvirt */
+ } else if (STREQ(arg, "-device")) {
+ /* something we can't fully parse. Check if supported and
+ * add it to the qemu namespace
+ * cmdline/environment advanced options and hope for the best
+ */
+ bool unsupported = false;
+ WANT_VALUE()
+
+ if ((def->os.arch == VIR_ARCH_PPC64) &&
+ def->os.machine && STREQ(def->os.machine, "pseries")) {
+ int nkws;
+ char **kws;
+ char **vals;
+ size_t j;
+
+ if (!qemuParseKeywords(val, &kws, &vals, &nkws, 1)) {
+ for (j = 0; j < nkws; j++) {
+ if (STREQLEN(kws[j], "ide", 3) ||
+ (STREQ(kws[j], "if") && vals[j] &&
+ STREQ(vals[j], "ide"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("pseries systems do not support ide devices '%s'"), val);
+ unsupported = true;
+ break;
+ }
+ }
+ for (j = 0; j < nkws; j++) {
+ VIR_FREE(kws[j]);
+ VIR_FREE(vals[j]);
+ }
+ VIR_FREE(kws);
+ VIR_FREE(vals);
+ }
+ }
+
+ if (unsupported || VIR_REALLOC_N(cmd->args, cmd->num_args+2) < 0)
+ goto error;
+ if (VIR_STRDUP(cmd->args[cmd->num_args++], arg) < 0)
+ goto error;
+ if (VIR_STRDUP(cmd->args[cmd->num_args++], val) < 0)
+ goto error;
} else {
/* something we can't yet parse. Add it to the qemu namespace
* cmdline/environment advanced options and hope for the best
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 6dd8bb0..0bf4c37 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -249,6 +249,7 @@ mymain(void)
DO_TEST("hyperv");
DO_TEST("pseries-nvram");
+ DO_TEST("pseries-disk");
DO_TEST("nosharepages");
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args
new file mode 100644
index 0000000..5fc0938
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args
@@ -0,0 +1,5 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-S -M pseries -m 512 -smp 1 \
+-no-acpi -boot c -usb \
+-boot c -hda /dev/HostVG/QEMUGuest1 -cdrom /root/boot.iso
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
new file mode 100644
index 0000000..dbbd6aa
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>87eedafe-eedc-4336-8130-ed9fe5dc90c8</uuid>
+ <memory unit='KiB'>524288</memory>
+ <currentMemory unit='KiB'>524288</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='ppc64' machine='pseries'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='scsi'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <disk type='file' device='cdrom'>
+ <driver name='qemu' type='raw'/>
+ <source file='/root/boot.iso'/>
+ <target dev='hdc' bus='scsi'/>
+ <readonly/>
+ <address type='drive' controller='0' bus='0' target='0' unit='2'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='scsi' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <graphics type='sdl'/>
+ <video>
+ <model type='cirrus' vram='9216' heads='1'/>
+ </video>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
11 years, 1 month
[libvirt] Planning for next release
by Daniel Veillard
If we want to land a release on Mon Dec 2, I would suggest entering
freeze next week. The amount of patches since 1.1.4 is not very large
so we could start the freeze say on Wed 27, but IMHO it all depennds if
Dan patches about splitting out the python bindings gets in for that
release. If yes I would probably prefer to freeze one more day (start
Tues 26), and bump release name to 1.2.0 as this is a significant change
from an user perspective, otherwise 1.1.5 and freeze next Wed.
Opinions ? Dan do you think you can/want to land this set in the
coming week ?
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
11 years, 1 month
[libvirt] [PATCH] Mostly revert "python: remove virConnectGetCPUModelNames from globals"
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This reverts commit 6b90d7428d72e92db292a9228c44701bfd5003c9.
The original problem was that libvirt_virConnectGetCPUModelNames
was listed twice in the exports table, once automatically from
the generator and once from the manual override. We merely needed
to list it in the skip_impl list, and not delete the manually
written code entirely.
---
python/libvirt-override.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 2f520c1..6546dd1 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -2276,6 +2276,58 @@ libvirt_virConnectGetVersion(PyObject *self ATTRIBUTE_UNUSED,
return PyInt_FromLong(hvVersion);
}
+PyObject *
+libvirt_virConnectGetCPUModelNames(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+ int c_retval;
+ virConnectPtr conn;
+ PyObject *rv = NULL, *pyobj_conn;
+ char **models = NULL;
+ size_t i;
+ int flags = 0;
+ const char *arch = NULL;
+
+ if (!PyArg_ParseTuple(args, (char *)"Osi:virConnectGetCPUModelNames",
+ &pyobj_conn, &arch, &flags))
+ return NULL;
+ conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+ LIBVIRT_BEGIN_ALLOW_THREADS;
+
+ c_retval = virConnectGetCPUModelNames(conn, arch, &models, flags);
+
+ LIBVIRT_END_ALLOW_THREADS;
+
+ if (c_retval == -1)
+ return VIR_PY_INT_FAIL;
+
+ if ((rv = PyList_New(c_retval)) == NULL)
+ goto error;
+
+ for (i = 0; i < c_retval; i++) {
+ PyObject *str;
+ if ((str = PyString_FromString(models[i])) == NULL)
+ goto error;
+
+ PyList_SET_ITEM(rv, i, str);
+ }
+
+done:
+ if (models) {
+ for (i = 0; i < c_retval; i++)
+ VIR_FREE(models[i]);
+ VIR_FREE(models);
+ }
+
+ return rv;
+
+error:
+ Py_XDECREF(rv);
+ rv = VIR_PY_INT_FAIL;
+ goto done;
+}
+
static PyObject *
libvirt_virConnectGetLibVersion(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args)
@@ -7176,6 +7228,7 @@ static PyMethodDef libvirtMethods[] = {
#include "libvirt-export.c"
{(char *) "virGetVersion", libvirt_virGetVersion, METH_VARARGS, NULL},
{(char *) "virConnectGetVersion", libvirt_virConnectGetVersion, METH_VARARGS, NULL},
+ {(char *) "virConnectGetCPUModelNames", libvirt_virConnectGetCPUModelNames, METH_VARARGS, NULL},
{(char *) "virConnectGetLibVersion", libvirt_virConnectGetLibVersion, METH_VARARGS, NULL},
{(char *) "virConnectOpenAuth", libvirt_virConnectOpenAuth, METH_VARARGS, NULL},
{(char *) "virConnectListDomainsID", libvirt_virConnectListDomainsID, METH_VARARGS, NULL},
--
1.8.3.1
11 years, 1 month
[libvirt] [PATCH] Don't start a nested job in qemuMigrationPrepareAny
by Ján Tomko
This nested job is canceled by the first ExitMonitor call (even though
it was not created by the corresponding EnterMonitor call), and
again in qemuMigrationPrepareAny if qemuProcessStart failed.
This can lead to a crash if the vm object was disposed of before calling
qemuDomainRemoveInactive:
0 ..62bc in virClassIsDerivedFrom (klass=0xdeadbeef,
parent=0x7ffce4cdd270) at util/virobject.c:166
1 ..6666 in virObjectIsClass at util/virobject.c:362
2 ..66b4 in virObjectLock at util/virobject.c:314
3 ..477e in virDomainObjListRemove at conf/domain_conf.c:2359
4 ..7a64 in qemuDomainRemoveInactive at qemu/qemu_domain.c:2087
5 ..956c in qemuMigrationPrepareAny at qemu/qemu_migration.c:2469
This was added by commit e4e2822, exposed by 5a4c237 and c7ac251.
https://bugzilla.redhat.com/show_bug.cgi?id=1018267
---
src/qemu/qemu_migration.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e87ea85..ef6f1c5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2358,10 +2358,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
goto endjob;
}
- if (qemuDomainObjBeginNestedJob(driver, vm,
- QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
- goto endjob;
-
/* Start the QEMU daemon, with the same command-line arguments plus
* -incoming $migrateFrom
*/
@@ -2370,8 +2366,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_QEMU_PROCESS_START_PAUSED |
VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) {
virDomainAuditStart(vm, "migrated", false);
- if (!qemuDomainObjEndJob(driver, vm))
- vm = NULL;
goto endjob;
}
@@ -2474,7 +2468,7 @@ stop:
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0);
endjob:
- if (vm && !qemuMigrationJobFinish(driver, vm)) {
+ if (!qemuMigrationJobFinish(driver, vm)) {
vm = NULL;
}
goto cleanup;
--
1.8.3.2
11 years, 1 month
[libvirt] [PATCH] util: Make port allocator data more up-to-date
by Martin Kletzander
There are few places where we know about port getting opened even
though it was not allocated on any virPortAllocator (or we know it was
reserved like this). We cannot notify the virPortAllocator that the
port is open because of that (since the port may be out of range) and
in that case, we are stuck with old data.
This also fixes the issue that if you have autoport='no' and
port='-1', the port never gets returned (introduced by 7b4a630484);
the case when we don't know whether to release the port or not.
There may be also other places where the port is being released even
though it might have been set statically out of range by the user. In
that case we report an error, but that shouldn't stop APIs releasing
ports (e.g. destroy). Moreover, the return value is not checked
sometimes and we're stuck with an error in the logs and also set as a
lastError.
To make this behave the best I can imagine, we'd have to have a single
virPortAllocator with configurable ranges (ports shared between
allocators), but that's an idea for another day. In this patch, I'm
modifyping the virPortAllocatorRelease() function to be safe for all
kinds of parameters, making it a no-op in case of out-of-range ports
and warning in case virBitmapClearBit() returns an error (which is
just a sanity code in combination with the range check).
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/libxl/libxl_driver.c | 13 ++++---------
src/qemu/qemu_process.c | 32 +++++++++++++++++++++-----------
src/util/virportallocator.c | 33 +++++++++------------------------
src/util/virportallocator.h | 4 ++--
tests/virportallocatortest.c | 5 +----
5 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 1b42f14..26e2d3a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -279,15 +279,10 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(false, driver->inhibitOpaque);
- if ((vm->def->ngraphics == 1) &&
- vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
- vm->def->graphics[0]->data.vnc.autoport) {
- vnc_port = vm->def->graphics[0]->data.vnc.port;
- if (vnc_port >= LIBXL_VNC_PORT_MIN) {
- if (virPortAllocatorRelease(driver->reservedVNCPorts,
- vnc_port) < 0)
- VIR_DEBUG("Could not mark port %d as unused", vnc_port);
- }
+ if (vm->def->ngraphics == 1 &&
+ vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
+ virPortAllocatorRelease(driver->reservedVNCPorts,
+ vm->def->graphics[0]->data.vnc.port);
}
/* Remove any cputune settings */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a26c079..dfaf95f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4331,25 +4331,35 @@ retry:
qemuProcessRemoveDomainStatus(driver, vm);
- /* Remove VNC and Spice ports from port reservation bitmap, but only if
- they were reserved by the driver (autoport=yes)
- */
+ /* Remove remote ports from all port allocators, even when
+ * autoport isn't set, because they might have been auto-allocated
+ * anyway. It also helps keeping the allocator data a tad more
+ * updated.
+ */
for (i = 0; i < vm->def->ngraphics; ++i) {
virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
- if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
- if (graphics->data.vnc.autoport) {
- virPortAllocatorRelease(driver->remotePorts,
- graphics->data.vnc.port);
- }
+ switch ((enum virDomainGraphicsType) graphics->type) {
+ case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+ virPortAllocatorRelease(driver->remotePorts,
+ graphics->data.vnc.port);
virPortAllocatorRelease(driver->webSocketPorts,
graphics->data.vnc.websocket);
- }
- if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
- graphics->data.spice.autoport) {
+ break;
+
+ case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
virPortAllocatorRelease(driver->remotePorts,
graphics->data.spice.port);
virPortAllocatorRelease(driver->remotePorts,
graphics->data.spice.tlsPort);
+ break;
+
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+ case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+ case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
+ /* Nothing has been allocated */
+
+ case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+ break;
}
}
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 694f191..f7f3755 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -27,11 +27,12 @@
#include "viralloc.h"
#include "virbitmap.h"
-#include "virportallocator.h"
-#include "virthread.h"
#include "virerror.h"
#include "virfile.h"
+#include "virlog.h"
+#include "virportallocator.h"
#include "virstring.h"
+#include "virthread.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -172,33 +173,17 @@ cleanup:
return ret;
}
-int virPortAllocatorRelease(virPortAllocatorPtr pa,
- unsigned short port)
+void virPortAllocatorRelease(virPortAllocatorPtr pa,
+ unsigned short port)
{
- int ret = -1;
-
if (!port)
- return 0;
+ return;
virObjectLock(pa);
- if (port < pa->start ||
- port > pa->end) {
- virReportInvalidArg(port, "port %d must be in range (%d, %d)",
- port, pa->start, pa->end);
- goto cleanup;
- }
-
- if (virBitmapClearBit(pa->bitmap,
- port - pa->start) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to release port %d"),
- port);
- goto cleanup;
- }
+ if (port >= pa->start && port <= pa->end &&
+ virBitmapClearBit(pa->bitmap, port - pa->start) < 0)
+ VIR_WARN("Problem releasing port %d from range '%s'", port, pa->name);
- ret = 0;
-cleanup:
virObjectUnlock(pa);
- return ret;
}
diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h
index c8aa6de..ba7c504 100644
--- a/src/util/virportallocator.h
+++ b/src/util/virportallocator.h
@@ -35,7 +35,7 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
int virPortAllocatorAcquire(virPortAllocatorPtr pa,
unsigned short *port);
-int virPortAllocatorRelease(virPortAllocatorPtr pa,
- unsigned short port);
+void virPortAllocatorRelease(virPortAllocatorPtr pa,
+ unsigned short port);
#endif /* __VIR_PORT_ALLOCATOR_H__ */
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 721356e..0d41c98 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -165,10 +165,7 @@ static int testAllocReuse(const void *args ATTRIBUTE_UNUSED)
goto cleanup;
}
-
- if (virPortAllocatorRelease(alloc, p2) < 0)
- goto cleanup;
-
+ virPortAllocatorRelease(alloc, p2);
if (virPortAllocatorAcquire(alloc, &p4) < 0)
goto cleanup;
if (p4 != 5902) {
--
1.8.4.3
11 years, 1 month
[libvirt] [PATCH] spec: Don't save/restore running VMs on libvirt-client update
by Cole Robinson
Restarting an active libvirt-guests.service is the equivalent of
doing:
/usr/libexec/libvirt-guests.sh stop
/usr/libexec/libvirt-guests.sh start
Which in a default configuration will managedsave every running VM,
and then restore them. Certainly not something we should do every
time the libvirt-client RPM is updated.
Just drop the try-restart attempt, I don't know what purpose it
serves anyways.
https://bugzilla.redhat.com/show_bug.cgi?id=962225
---
libvirt.spec.in | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index a5b01df..0d50ccd 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1714,9 +1714,8 @@ fi
# If the package is allowed to autostart:
/bin/systemctl --no-reload enable libvirt-guests.service >/dev/null 2>&1 ||:
-# Run these because the SysV package being removed won't do them
+# Run this because the SysV package being removed won't do them
/sbin/chkconfig --del libvirt-guests >/dev/null 2>&1 || :
-/bin/systemctl try-restart libvirt-guests.service >/dev/null 2>&1 || :
%endif
%if %{with_sanlock}
--
1.8.4.2
11 years, 1 month
[libvirt] [PATCH] spec: Restrict virt-login-shell usage
by Jiri Denemark
https://bugzilla.redhat.com/show_bug.cgi?id=1033614
As virt-login-shell is an SUID binary, we should restrict its usage to
just the users chosen by an administrator to use virt-login-shell as
their login shell. This can easily be done by making the binary
executable only by users from a new virtlogin group.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
libvirt.spec.in | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index a5b01df..864fbf4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1727,6 +1727,12 @@ if getent group sanlock > /dev/null ; then
fi
%endif
+%if %{with_lxc}
+%pre login-shell
+getent group virtlogin >/dev/null || groupadd -r virtlogin
+exit 0
+%endif
+
%files
%defattr(-, root, root)
@@ -2072,7 +2078,7 @@ fi
%if %{with_lxc}
%files login-shell
-%attr(4755, root, root) %{_bindir}/virt-login-shell
+%attr(4750, root, virtlogin) %{_bindir}/virt-login-shell
%config(noreplace) %{_sysconfdir}/libvirt/virt-login-shell.conf
%{_mandir}/man1/virt-login-shell.1*
%endif
--
1.8.4.4
11 years, 1 month
[libvirt] [PATCH 0/2] Network events feature
by Cédric Bosdonnat
These changes are all about bringing events for network object like the
ones existing for domains. This feature is needed for virt-manager to
refresh its UI when networks are started/destroyed/defined/undefined.
The first commit is a refactoring of the domain events internal functions
to extract the reusable parts of them. The second commit adds network
events based on those now-common event functions. The network events
are implemented in the test and bridge network drivers ATM.
Cédric Bosdonnat (2):
virDomainEvent refactoring to separate generic things from domain ones
Added network events API and unit tests
.gitignore | 2 +
daemon/libvirtd.h | 1 +
daemon/remote.c | 175 +++-
include/libvirt/libvirt.h.in | 85 ++
python/generator.py | 2 +
python/libvirt-override-virConnect.py | 34 +
python/libvirt-override.c | 150 ++++
src/Makefile.am | 6 +
src/conf/domain_event.c | 1431 ++++++---------------------------
src/conf/domain_event.h | 134 ++-
src/conf/object_event.c | 1008 +++++++++++++++++++++++
src/conf/object_event.h | 114 +++
src/conf/object_event_private.h | 167 ++++
src/driver.h | 14 +
src/libvirt.c | 133 +++
src/libvirt_private.syms | 18 +-
src/libvirt_public.syms | 6 +
src/libxl/libxl_conf.h | 2 +-
src/libxl/libxl_driver.c | 28 +-
src/lxc/lxc_conf.h | 2 +-
src/lxc/lxc_driver.c | 38 +-
src/lxc/lxc_process.c | 12 +-
src/network/bridge_driver.c | 91 ++-
src/network/bridge_driver_platform.h | 3 +
src/parallels/parallels_utils.h | 2 +-
src/qemu/qemu_conf.h | 2 +-
src/qemu/qemu_domain.c | 6 +-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 50 +-
src/qemu/qemu_hotplug.c | 10 +-
src/qemu/qemu_migration.c | 12 +-
src/qemu/qemu_process.c | 48 +-
src/remote/remote_driver.c | 173 +++-
src/remote/remote_protocol.x | 46 +-
src/test/test_driver.c | 166 ++--
src/uml/uml_conf.h | 2 +-
src/uml/uml_driver.c | 26 +-
src/vbox/vbox_tmpl.c | 20 +-
src/xen/xen_driver.c | 10 +-
src/xen/xen_driver.h | 4 +-
src/xen/xen_inotify.c | 8 +-
src/xen/xs_internal.c | 4 +-
tests/Makefile.am | 7 +
tests/objecteventtest.c | 228 ++++++
tests/qemuhotplugtest.c | 2 +-
45 files changed, 2974 insertions(+), 1510 deletions(-)
create mode 100644 src/conf/object_event.c
create mode 100644 src/conf/object_event.h
create mode 100644 src/conf/object_event_private.h
create mode 100644 tests/objecteventtest.c
--
1.8.4.2
11 years, 1 month
