Re: [libvirt] [PATCH v4 6/6] qemu-command: introduce new vgamem attribute for QXL video device
Pavel Hrdina writes: > Well, I think that this is not correct usage of the libvirt XML. You > should not just change one part of the device configuration. If user of > virt-manager change the device type, you should create a new XML > according to the new device type and copy all valid values and then drop > the original XML configuration. > > I don't think that just silently remove unsupported configuration is > appropriate solution. Even if we will print warning that it's not > supported and it will be removed I still don't like it. ok thanks to have confirmed it. I'll fix it in virt-manager. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 6/6] qemu-command: introduce new vgamem attribute for QXL video device
Pavel Hrdina writes: > The vgamem is a new attribute for QXL. The vram stil exists and its > only silently updated to proper value. The configuration with vram=9216 > is wrong and should be updated in virt-manager tests. the issue exists when the type of an existing QXL device is changed. virt-manager re-uses the XML configuration without modifying attributes it doesn't understand (as vgamem in this case) but changing only type='qxl' to type='$NEW_TYPE'. My doubt here is if an additional attribute should be silently ignored or users of the API should be filtering it out if type != 'qxl'. Another point is that this attribute will be added in future for some other models, then all users must be updated as well to support it, instead of having it only in libvirt. What do you think? Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 6/6] qemu-command: introduce new vgamem attribute for QXL video device
Pavel Hrdina writes: > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 6313d30..5c1b1de 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -10272,6 +10272,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, > char *heads = NULL; > char *vram = NULL; > char *ram = NULL; > +char *vgamem = NULL; > char *primary = NULL; > > if (VIR_ALLOC(def) < 0) > @@ -10285,6 +10286,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, > type = virXMLPropString(cur, "type"); > ram = virXMLPropString(cur, "ram"); > vram = virXMLPropString(cur, "vram"); > +vgamem = virXMLPropString(cur, "vgamem"); > heads = virXMLPropString(cur, "heads"); > > if ((primary = virXMLPropString(cur, "primary")) != NULL) { > @@ -10338,6 +10340,19 @@ virDomainVideoDefParseXML(xmlNodePtr node, > def->vram = virDomainVideoDefaultRAM(dom, def->type); > } > > +if (vgamem) { > +if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { > +virReportError(VIR_ERR_XML_ERROR, "%s", > + _("vgamem attribute only supported for type of > qxl")); > +goto error; Should libvirt ignore XML attributes that makes no sense for some models and keep accepting a configuration that used to work before? (I am replying to this old post, just because it caused a regression in virt-manager) Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCH] rpc: do not fail if the pid of the connecting process is not set
Martin Kletzander writes: > On Wed, Nov 19, 2014 at 06:29:48PM +0100, Giuseppe Scrivano wrote: >>getsockopt(sock->fd, SOL_SOCKET, SO_PEERCRED, ...) sets the pid to 0 >>when the process that opens the connection is in another container. >> >>Signed-off-by: Giuseppe Scrivano >>--- >> src/rpc/virnetsocket.c | 8 ++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >>diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c >>index 882fbec..6b019cc 100644 >>--- a/src/rpc/virnetsocket.c >>+++ b/src/rpc/virnetsocket.c >>@@ -1251,10 +1251,14 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, >> goto cleanup; >> } >> >>-if (virProcessGetStartTime(cr.pid, timestamp) < 0) >>+*timestamp = -1; >>+if (cr.pid && virProcessGetStartTime(cr.pid, timestamp) < 0) >> goto cleanup; >> >>-*pid = cr.pid; >>+if (cr.pid) >>+*pid = cr.pid; >>+else >>+*pid = -1; >> *uid = cr.uid; >> *gid = cr.gid; >> >>-- >>1.9.3 >> > > ACK, > > Martin Thanks, pushed now. Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt PATCH] rpc: do not fail if the pid of the connecting process is not set
getsockopt(sock->fd, SOL_SOCKET, SO_PEERCRED, ...) sets the pid to 0 when the process that opens the connection is in another container. Signed-off-by: Giuseppe Scrivano --- src/rpc/virnetsocket.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 882fbec..6b019cc 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1251,10 +1251,14 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, goto cleanup; } -if (virProcessGetStartTime(cr.pid, timestamp) < 0) +*timestamp = -1; +if (cr.pid && virProcessGetStartTime(cr.pid, timestamp) < 0) goto cleanup; -*pid = cr.pid; +if (cr.pid) +*pid = cr.pid; +else +*pid = -1; *uid = cr.uid; *gid = cr.gid; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: raise an error when trying to use readonly sata disks
Martin Kletzander writes: > On Mon, Sep 22, 2014 at 04:02:49PM +0200, Giuseppe Scrivano wrote: >>commit 72f919f558902968bd0cf9f99f25ac62cbfe3ac6 introduced an user >>friendly error message when trying to use IDE disks as readonly. >> >>Do the same thing for the SATA bus. >> >>Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939 >> >>Signed-off-by: Giuseppe Scrivano >>--- >> src/qemu/qemu_command.c | 16 +++- >> 1 file changed, 11 insertions(+), 5 deletions(-) >> > > ACK, thanks, pushed now. Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: raise an error when trying to use readonly sata disks
commit 72f919f558902968bd0cf9f99f25ac62cbfe3ac6 introduced an user friendly error message when trying to use IDE disks as readonly. Do the same thing for the SATA bus. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939 Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 86e0290..ca1b6cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3508,11 +3508,17 @@ qemuBuildDriveStr(virConnectPtr conn, virBufferAddLit(&opt, ",boot=on"); if (disk->src->readonly && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) { -if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE && -disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("readonly ide disks are not supported")); -goto error; +if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { +if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("readonly ide disks are not supported")); +goto error; +} +if (disk->bus == VIR_DOMAIN_DISK_BUS_SATA) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("readonly sata disks are not supported")); +goto error; +} } virBufferAddLit(&opt, ",readonly=on"); } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: force FIPS testing mode with new enough GNU TLS versions
"Daniel P. Berrange" writes: > ACK thanks, pushed both patches. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: force FIPS testing mode with new enough GNU TLS versions
Signed-off-by: Giuseppe Scrivano --- tests/virnettlscontexttest.c | 2 ++ tests/virnettlssessiontest.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 51a0369..a3e24a3 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -113,6 +113,8 @@ mymain(void) { int ret = 0; +setenv("GNUTLS_FORCE_FIPS_MODE", "2", 1); + testTLSInit(KEYFILE); # define DO_CTX_TEST(_isServer, _caCrt, _crt, _expectFail) \ diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index 1e2683c..3af948a 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -240,6 +240,8 @@ mymain(void) { int ret = 0; +setenv("GNUTLS_FORCE_FIPS_MODE", "2", 1); + testTLSInit(KEYFILE); # define DO_SESS_TEST(_caCrt, _serverCrt, _clientCrt, _expectServerFail, \ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] security: fix DH key generation when FIPS mode is on
"Daniel P. Berrange" writes: > On Thu, Sep 04, 2014 at 10:33:37AM +0200, Giuseppe Scrivano wrote: >> When FIPS mode is on, gnutls_dh_params_generate2 will fail if 1024 is >> specified as the prime's number of bits, a bigger value works in both >> cases. >> >> Signed-off-by: Giuseppe Scrivano >> --- >> >> with the development version of GNU TLS is possible to test FIPS mode >> setting the env variable GNUTLS_FORCE_FIPS_MODE=2 > > How about we set that env variable in our two TLS tests too, because > we really want libvirt to be always able to run in FIPS mode. sure, I will send a follow-up patch. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] security: fix DH key generation when FIPS mode is on
When FIPS mode is on, gnutls_dh_params_generate2 will fail if 1024 is specified as the prime's number of bits, a bigger value works in both cases. Signed-off-by: Giuseppe Scrivano --- with the development version of GNU TLS is possible to test FIPS mode setting the env variable GNUTLS_FORCE_FIPS_MODE=2 src/rpc/virnettlscontext.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 31aac9d..947038d 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -43,7 +43,7 @@ #include "virthread.h" #include "configmake.h" -#define DH_BITS 1024 +#define DH_BITS 2048 #define LIBVIRT_PKI_DIR SYSCONFDIR "/pki" #define LIBVIRT_CACERT LIBVIRT_PKI_DIR "/CA/cacert.pem" -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 libvirt 3/3] qemu: add support for MTP filesystem
Michal Privoznik writes: > Question that pops up immediately: do we want to commit to something > that even qemu developers don't believe yet? What will happen when > qemu decides to switch to 'root' attribute again? Libvirt will have to > adapt which won't work with older qemus supporting 'x-root' only. I am not sure how usually libvirt deals with these cases, but to me it seems like that we should use the "stable" attribute name; or just postpone this series/feature. > > And what's this desc= attribute for anyway? I've put in several > different values and it seems to me like nothing changed. it is the name given to the MTP share on the guest, I see it displayed in the guest, at least Nautilus shows it. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 libvirt 1/3] conf: add child element to
"Daniel P. Berrange" writes: > From the XML pov, we shouldn't restrict use of to only be > for FS_TYPE_MOUNT. That is a QEMU driver implementation restriction, > so just make the QEMU driver raise VIR_ERR_CONFIG_UNSUPPORTED for > the cases we don't want, when building the CLI args OK to amend this piece? The QEMU driver supports only FS_TYPE_MOUNT so the additional check seems reduntant there. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 98dbe14..c897f1b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15806,10 +15806,6 @@ virDomainFSDefFormat(virBufferPtr buf, case VIR_DOMAIN_FS_TYPE_MOUNT: virBufferEscapeString(buf, "\n", def->src); -if (def->model) { -virBufferEscapeString(buf, "\n", - virDomainFSModelTypeToString(def->model)); -} break; case VIR_DOMAIN_FS_TYPE_BIND: @@ -15838,6 +15834,11 @@ virDomainFSDefFormat(virBufferPtr buf, break; } +if (def->model) { +virBufferEscapeString(buf, "\n", + virDomainFSModelTypeToString(def->model)); +} + virBufferEscapeString(buf, "\n", def->dst); Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 libvirt 1/3] conf: add child element to
Signed-off-by: Giuseppe Scrivano --- docs/formatdomain.html.in | 6 ++ docs/schemas/domaincommon.rng | 13 + src/conf/domain_conf.c | 26 ++ src/conf/domain_conf.h | 11 +++ tests/domainconfdata/getfilesystem.xml | 5 + tests/domainconftest.c | 1 + 6 files changed, 62 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ed17389..d0c1ce2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2300,6 +2300,8 @@ <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='/import/from/host'/> + <model type='9p'/> + <model type='mtp'/> <readonly/> </filesystem> <filesystem type='file' accessmode='passthrough'> @@ -2337,6 +2339,10 @@ while the value immediate means that a host writeback is immediately triggered for all pages touched during a guest file write operation (since 0.9.10). +A "filesystem" element has an optional +attribute model (since +1.2.8), which is one of "9p", "mtp" (used by QEMU/KVM), +if this element is not specified the default is "9p". type='template' diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 033f2f6..684acec 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1937,6 +1937,19 @@ + + + + + +9p +mtp + + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 14338ba..98dbe14 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -344,6 +344,11 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "ram", "bind") +VIR_ENUM_IMPL(virDomainFSModel, VIR_DOMAIN_FS_MODEL_LAST, + "default", + "9p", + "mtp") + VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", "path", @@ -6459,6 +6464,7 @@ virDomainFSDefParseXML(xmlNodePtr node, virDomainFSDefPtr def; xmlNodePtr cur, save_node = ctxt->node; char *type = NULL; +char *model = NULL; char *fsdriver = NULL; char *source = NULL; char *target = NULL; @@ -6536,6 +6542,9 @@ virDomainFSDefParseXML(xmlNodePtr node, wrpolicy = virXMLPropString(cur, "wrpolicy"); if (!format) format = virXMLPropString(cur, "format"); +} else if (!model && + xmlStrEqual(cur->name, BAD_CAST "model")) { +model = virXMLPropString(cur, "type"); } } cur = cur->next; @@ -6557,6 +6566,14 @@ virDomainFSDefParseXML(xmlNodePtr node, } } +if (model) { +if ((def->model = virDomainFSModelTypeFromString(model)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown model value '%s'"), model); +goto error; +} +} + if (wrpolicy) { if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -6616,6 +6633,7 @@ virDomainFSDefParseXML(xmlNodePtr node, VIR_FREE(usage); VIR_FREE(units); VIR_FREE(format); +VIR_FREE(model); return def; @@ -15786,6 +15804,14 @@ virDomainFSDefFormat(virBufferPtr buf, switch (def->type) { case VIR_DOMAIN_FS_TYPE_MOUNT: +virBufferEscapeString(buf, "\n", + def->src); +if (def->model) { +virBufferEscapeString(buf, "\n", + virDomainFSModelTypeToString(def->model)); +} +break; + case VIR_DOMAIN_FS_TYPE_BIND: virBufferEscapeString(buf, "\n", def->src); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4cf56c9..df7d019 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -764,6 +764,15 @@ typedef enum { VIR_DOMAIN_FS_TYPE_LAST } virDomainFSType; +/* Filesystem model */ +typedef enum { +VIR_DOMAIN_FS_MODEL_DEFAULT = 0, +VIR_DOMAIN_FS_MODEL_9P, /* 9P network filesystem */ +VIR_DOMAIN_FS_MODEL_MTP,/* MTP usb filesystem */ + +VIR_DOMAIN_FS_MO
[libvirt] [PATCH v3 libvirt 2/3] docs: relax / dir element attribute to string
It is used by mtp model type to specify the mtp share name. Signed-off-by: Giuseppe Scrivano --- docs/schemas/domaincommon.rng | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 684acec..4178644 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1899,9 +1899,7 @@ - - - + -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 libvirt 3/3] qemu: add support for MTP filesystem
Generate the qemu command line option: -device 'usb-mtp,root=$SRC,desc=$TARGET' from the definition XML: Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c| 65 -- tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args | 6 +++ tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml | 30 3 files changed, 77 insertions(+), 24 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b68695d..850fd71 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2060,8 +2060,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) continue; -/* Only support VirtIO-9p-pci so far. If that changes, - * we might need to skip devices here */ +if (def->fss[i]->model == VIR_DOMAIN_FS_MODEL_MTP) +continue; + if (virDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info, flags) < 0) goto error; @@ -3956,12 +3957,6 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only supports mount filesystem type")); -goto error; -} - if (!driver) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Filesystem driver type not supported")); @@ -4030,22 +4025,36 @@ qemuBuildFSDevStr(virDomainDefPtr def, { virBuffer opt = VIR_BUFFER_INITIALIZER; -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("can only passthrough directories")); -goto error; -} +if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { -virBufferAddLit(&opt, "virtio-9p-pci"); -virBufferAsprintf(&opt, ",id=%s", fs->info.alias); -virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); -virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); +switch ((virDomainFSModel) fs->model) { +case VIR_DOMAIN_FS_MODEL_MTP: +virBufferAddLit(&opt, "usb-mtp"); +virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src, fs->dst); +break; +case VIR_DOMAIN_FS_MODEL_DEFAULT: +case VIR_DOMAIN_FS_MODEL_9P: +virBufferAddLit(&opt, "virtio-9p-pci"); +virBufferAsprintf(&opt, ",id=%s", fs->info.alias); +virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, + fs->info.alias); +virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); +if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) +goto error; +break; -if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) -goto error; +case VIR_DOMAIN_FS_MODEL_LAST: +/* nada */ +break; +} -if (virBufferCheckError(&opt) < 0) +if (virBufferCheckError(&opt) < 0) +goto error; +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported filesystem type")); goto error; +} return virBufferContentAndReset(&opt); @@ -8320,11 +8329,19 @@ qemuBuildCommandLine(virConnectPtr conn, char *optstr; virDomainFSDefPtr fs = def->fss[i]; -virCommandAddArg(cmd, "-fsdev"); -if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) +if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports mount filesystem type")); goto error; -virCommandAddArg(cmd, optstr); -VIR_FREE(optstr); +} + +if (fs->model != VIR_DOMAIN_FS_MODEL_MTP) { +virCommandAddArg(cmd, "-fsdev"); +if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) +goto error; +virCommandAddArg(cmd, optstr); +VIR_FREE(optstr); +}
[libvirt] [PATCH v3 libvirt 0/3] Add support for qemu usb-mtp device
This series adds support for qemu usb-mtp devices. This new version addresses comments for v2: https://www.redhat.com/archives/libvir-list/2014-August/msg00471.html Giuseppe Scrivano (3): conf: add child element to docs: relax / dir element attribute to string qemu: add support for MTP filesystem docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 17 +-- src/conf/domain_conf.c | 26 +++ src/conf/domain_conf.h | 11 + src/qemu/qemu_command.c| 65 -- tests/domainconfdata/getfilesystem.xml | 5 ++ tests/domainconftest.c | 1 + tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args | 6 +++ tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml | 30 9 files changed, 140 insertions(+), 27 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 libvirt 7/8] conf: fix comment
Michal Privoznik writes: > On 11.08.2014 16:47, Giuseppe Scrivano wrote: >> Signed-off-by: Giuseppe Scrivano >> --- >> src/conf/domain_conf.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h >> index 748dea7..32ca32d 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >> @@ -752,7 +752,7 @@ struct _virDomainControllerDef { >> }; >> >> >> -/* Two types of disk backends */ >> +/* Types of disk backends */ >> typedef enum { >> VIR_DOMAIN_FS_TYPE_MOUNT, /* Mounts (binds) a host dir on a guest dir >> */ >> VIR_DOMAIN_FS_TYPE_BLOCK, /* Mounts a host block dev on a guest dir */ >> > > ACK, again can be pushed out of order. thanks, I am going to push it soon. Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 libvirt 4/8] qemu: add support for MTP filesystem
Michal Privoznik writes: > On 11.08.2014 16:47, Giuseppe Scrivano wrote: >> Generate the qemu command line option: >> >> -device 'usb-mtp,root=$SRC,desc=$TARGET' >> >> from the definition XML: >> >> >> >> >> >> >> >> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 >> >> Signed-off-by: Giuseppe Scrivano >> --- >> src/qemu/qemu_command.c | 60 >> - >> 1 file changed, 35 insertions(+), 25 deletions(-) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 87569b1..07f165e 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -2060,8 +2060,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, >> if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) >> continue; >> >> -/* Only support VirtIO-9p-pci so far. If that changes, >> - * we might need to skip devices here */ >> +if (def->fss[i]->model == VIR_DOMAIN_FS_MODEL_MTP) >> +continue; >> + >> if (virDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info, >> flags) < 0) >> goto error; >> @@ -3956,12 +3957,6 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, >> const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); >> const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); >> >> -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { >> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> - _("only supports mount filesystem type")); >> -goto error; >> -} >> - >> if (!driver) { >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> _("Filesystem driver type not supported")); >> @@ -4030,22 +4025,28 @@ qemuBuildFSDevStr(virDomainDefPtr def, >> { >> virBuffer opt = VIR_BUFFER_INITIALIZER; >> >> -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { >> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> - _("can only passthrough directories")); >> -goto error; >> -} >> - >> -virBufferAddLit(&opt, "virtio-9p-pci"); >> -virBufferAsprintf(&opt, ",id=%s", fs->info.alias); >> -virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, >> fs->info.alias); >> -virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); >> +if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { >> >> -if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) >> -goto error; >> +if (fs->model == VIR_DOMAIN_FS_MODEL_MTP) { > > I'd feel safer with: > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index ac8803a..ec269d6 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4027,10 +4027,13 @@ qemuBuildFSDevStr(virDomainDefPtr def, > > if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { > > -if (fs->model == VIR_DOMAIN_FS_MODEL_MTP) { > +switch ((virDomainFSModel) fs->model) { > +case VIR_DOMAIN_FS_MODEL_MTP: > virBufferAddLit(&opt, "usb-mtp"); > virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src, fs->dst); > -} else { > +break; > +case VIR_DOMAIN_FS_MODEL_DEFAULT: > +case VIR_DOMAIN_FS_MODEL_9P: > virBufferAddLit(&opt, "virtio-9p-pci"); > virBufferAsprintf(&opt, ",id=%s", fs->info.alias); > virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, > @@ -4038,6 +4041,11 @@ qemuBuildFSDevStr(virDomainDefPtr def, > virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); > if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < > 0) > goto error; > +break; > + > +case VIR_DOMAIN_FS_MODEL_LAST: > +/* nada */ > +break; > } > > if (virBufferCheckError(&opt) < 0) > >> +virBufferAddLit(&opt, "usb-mtp"); >> +virBufferAsprintf(&opt, "
Re: [libvirt] [PATCH v2 libvirt 3/8] qemu_command: fix block indentation
Michal Privoznik writes: > On 11.08.2014 16:47, Giuseppe Scrivano wrote: >> Signed-off-by: Giuseppe Scrivano >> --- >> src/qemu/qemu_command.c | 14 +++--- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 8a69976..87569b1 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -3989,13 +3989,13 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, >> } >> >> if (fs->wrpolicy) { >> - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { >> - virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); >> - } else { >> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> - _("filesystem writeout not supported")); >> - goto error; >> - } >> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { >> +virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); >> +} else { >> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("filesystem writeout not supported")); >> +goto error; >> +} >> } >> >> virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, >> fs->info.alias); >> > > ACK. Again independent patch. going to push it soon. Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 libvirt 1/8] conf: add child element to
Wang Rui writes: > On 2014/8/11 22:47, Giuseppe Scrivano wrote: > >> @@ -6458,6 +6463,7 @@ virDomainFSDefParseXML(xmlNodePtr node, >> virDomainFSDefPtr def; >> xmlNodePtr cur, save_node = ctxt->node; >> char *type = NULL; >> +char *model = NULL; >> char *fsdriver = NULL; >> char *source = NULL; >> char *target = NULL; >> @@ -6535,6 +6541,9 @@ virDomainFSDefParseXML(xmlNodePtr node, >> wrpolicy = virXMLPropString(cur, "wrpolicy"); >> if (!format) >> format = virXMLPropString(cur, "format"); >> +} else if (!model && >> + xmlStrEqual(cur->name, BAD_CAST "model")) { >> +model = virXMLPropString(cur, "type"); >> } >> } >> cur = cur->next; >> @@ -6556,6 +6565,14 @@ virDomainFSDefParseXML(xmlNodePtr node, >> } >> } >> >> +if (model) { >> +if ((def->model = virDomainFSModelTypeFromString(model)) <= 0) { >> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >> + _("unknown model value '%s'"), model); >> +goto error; >> +} >> +} >> + >> if (wrpolicy) { >> if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) >> <= 0) { >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > > The memory allocated by model should be freed in cleanup. ping? Any other comment on this series? Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 5/8] docs: add documentation and schema for /
Signed-off-by: Giuseppe Scrivano --- docs/formatdomain.html.in | 6 ++ docs/schemas/domaincommon.rng | 13 + 2 files changed, 19 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b5df51..c2d04a9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2300,6 +2300,8 @@ <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='/import/from/host'/> + <model type='9p'/> + <model type='mtp'/> <readonly/> </filesystem> <filesystem type='file' accessmode='passthrough'> @@ -2337,6 +2339,10 @@ while the value immediate means that a host writeback is immediately triggered for all pages touched during a guest file write operation (since 0.9.10). +A "filesystem" element has an optional +attribute model (since +1.2.8), which is one of "9p", "mtp" (used by QEMU/KVM), +if this element is not specified the default is "9p". type='template' diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 033f2f6..684acec 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1937,6 +1937,19 @@ + + + + + +9p +mtp + + + + + + -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 3/8] qemu_command: fix block indentation
Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a69976..87569b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3989,13 +3989,13 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, } if (fs->wrpolicy) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { - virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("filesystem writeout not supported")); - goto error; - } +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { +virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("filesystem writeout not supported")); +goto error; +} } virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 7/8] conf: fix comment
Signed-off-by: Giuseppe Scrivano --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 748dea7..32ca32d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -752,7 +752,7 @@ struct _virDomainControllerDef { }; -/* Two types of disk backends */ +/* Types of disk backends */ typedef enum { VIR_DOMAIN_FS_TYPE_MOUNT, /* Mounts (binds) a host dir on a guest dir */ VIR_DOMAIN_FS_TYPE_BLOCK, /* Mounts a host block dev on a guest dir */ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 2/8] conf, virDomainFSDefPtr: rename "path" argument to "target"
Since the target for MTP is a name and not a path, make the function more generic. Signed-off-by: Giuseppe Scrivano --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9252ffa..20de23a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18925,12 +18925,12 @@ virDomainFSRemove(virDomainDefPtr def, size_t i) virDomainFSDefPtr virDomainGetFilesystemForTarget(virDomainDefPtr def, -const char *path) +const char *target) { size_t i; for (i = 0; i < def->nfss; i++) { -if (STREQ(def->fss[i]->dst, path)) +if (STREQ(def->fss[i]->dst, target)) return def->fss[i]; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d7664e4..748dea7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2495,7 +2495,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk, int *devIdx); virDomainFSDefPtr virDomainGetFilesystemForTarget(virDomainDefPtr def, - const char *path); + const char *target); int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs); int virDomainFSIndexByName(virDomainDefPtr def, const char *name); virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 8/8] tests: add tests for filesystem model mtp
Signed-off-by: Giuseppe Scrivano --- tests/domainconfdata/getfilesystem.xml | 5 + tests/domainconftest.c | 1 + 2 files changed, 6 insertions(+) diff --git a/tests/domainconfdata/getfilesystem.xml b/tests/domainconfdata/getfilesystem.xml index 2ee78b4..3203666 100644 --- a/tests/domainconfdata/getfilesystem.xml +++ b/tests/domainconfdata/getfilesystem.xml @@ -21,6 +21,11 @@ + + + + + diff --git a/tests/domainconftest.c b/tests/domainconftest.c index 3d6ebe1..6b65f09 100644 --- a/tests/domainconftest.c +++ b/tests/domainconftest.c @@ -111,6 +111,7 @@ mymain(void) DO_TEST_GET_FS("/dev", true); DO_TEST_GET_FS("/dev/pts", false); DO_TEST_GET_FS("/doesnotexist", false); +DO_TEST_GET_FS("mtp share", true); virObjectUnref(caps); virObjectUnref(xmlopt); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 6/8] docs: relax / dir element attribute to string
It is used by mtp model type to specify the mtp share name. Signed-off-by: Giuseppe Scrivano --- docs/schemas/domaincommon.rng | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 684acec..4178644 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1899,9 +1899,7 @@ - - - + -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 4/8] qemu: add support for MTP filesystem
Generate the qemu command line option: -device 'usb-mtp,root=$SRC,desc=$TARGET' from the definition XML: Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 60 - 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 87569b1..07f165e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2060,8 +2060,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) continue; -/* Only support VirtIO-9p-pci so far. If that changes, - * we might need to skip devices here */ +if (def->fss[i]->model == VIR_DOMAIN_FS_MODEL_MTP) +continue; + if (virDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info, flags) < 0) goto error; @@ -3956,12 +3957,6 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only supports mount filesystem type")); -goto error; -} - if (!driver) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Filesystem driver type not supported")); @@ -4030,22 +4025,28 @@ qemuBuildFSDevStr(virDomainDefPtr def, { virBuffer opt = VIR_BUFFER_INITIALIZER; -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("can only passthrough directories")); -goto error; -} - -virBufferAddLit(&opt, "virtio-9p-pci"); -virBufferAsprintf(&opt, ",id=%s", fs->info.alias); -virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); -virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); +if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { -if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) -goto error; +if (fs->model == VIR_DOMAIN_FS_MODEL_MTP) { +virBufferAddLit(&opt, "usb-mtp"); +virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src, fs->dst); +} else { +virBufferAddLit(&opt, "virtio-9p-pci"); +virBufferAsprintf(&opt, ",id=%s", fs->info.alias); +virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, + fs->info.alias); +virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); +if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) +goto error; +} -if (virBufferCheckError(&opt) < 0) +if (virBufferCheckError(&opt) < 0) +goto error; +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported filesystem type")); goto error; +} return virBufferContentAndReset(&opt); @@ -8320,11 +8321,20 @@ qemuBuildCommandLine(virConnectPtr conn, char *optstr; virDomainFSDefPtr fs = def->fss[i]; -virCommandAddArg(cmd, "-fsdev"); -if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) +if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports mount filesystem type")); goto error; -virCommandAddArg(cmd, optstr); -VIR_FREE(optstr); +} + +if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT && +fs->model != VIR_DOMAIN_FS_MODEL_MTP) { +virCommandAddArg(cmd, "-fsdev"); +if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) +goto error; +virCommandAddArg(cmd, optstr); +VIR_FREE(optstr); +} virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps))) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 1/8] conf: add child element to
Signed-off-by: Giuseppe Scrivano --- src/conf/domain_conf.c | 25 + src/conf/domain_conf.h | 11 +++ 2 files changed, 36 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7016f3..9252ffa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -344,6 +344,11 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "ram", "bind") +VIR_ENUM_IMPL(virDomainFSModel, VIR_DOMAIN_FS_MODEL_LAST, + "default", + "9p", + "mtp") + VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", "path", @@ -6458,6 +6463,7 @@ virDomainFSDefParseXML(xmlNodePtr node, virDomainFSDefPtr def; xmlNodePtr cur, save_node = ctxt->node; char *type = NULL; +char *model = NULL; char *fsdriver = NULL; char *source = NULL; char *target = NULL; @@ -6535,6 +6541,9 @@ virDomainFSDefParseXML(xmlNodePtr node, wrpolicy = virXMLPropString(cur, "wrpolicy"); if (!format) format = virXMLPropString(cur, "format"); +} else if (!model && + xmlStrEqual(cur->name, BAD_CAST "model")) { +model = virXMLPropString(cur, "type"); } } cur = cur->next; @@ -6556,6 +6565,14 @@ virDomainFSDefParseXML(xmlNodePtr node, } } +if (model) { +if ((def->model = virDomainFSModelTypeFromString(model)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown model value '%s'"), model); +goto error; +} +} + if (wrpolicy) { if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -15795,6 +15812,14 @@ virDomainFSDefFormat(virBufferPtr buf, switch (def->type) { case VIR_DOMAIN_FS_TYPE_MOUNT: +virBufferEscapeString(buf, "\n", + def->src); +if (def->model) { +virBufferEscapeString(buf, "\n", + virDomainFSModelTypeToString(def->model)); +} +break; + case VIR_DOMAIN_FS_TYPE_BIND: virBufferEscapeString(buf, "\n", def->src); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ff7d640..d7664e4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -764,6 +764,15 @@ typedef enum { VIR_DOMAIN_FS_TYPE_LAST } virDomainFSType; +/* Filesystem model */ +typedef enum { +VIR_DOMAIN_FS_MODEL_DEFAULT = 0, +VIR_DOMAIN_FS_MODEL_9P, /* 9P network filesystem */ +VIR_DOMAIN_FS_MODEL_MTP, /* MTP usb filesystem */ + +VIR_DOMAIN_FS_MODEL_LAST +} virDomainFSModel; + /* Filesystem driver type */ typedef enum { VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT = 0, @@ -808,6 +817,7 @@ struct _virDomainFSDef { virDomainDeviceInfo info; unsigned long long space_hard_limit; /* in bytes */ unsigned long long space_soft_limit; /* in bytes */ +int model; }; @@ -2585,6 +2595,7 @@ VIR_ENUM_DECL(virDomainControllerModelPCI) VIR_ENUM_DECL(virDomainControllerModelSCSI) VIR_ENUM_DECL(virDomainControllerModelUSB) VIR_ENUM_DECL(virDomainFS) +VIR_ENUM_DECL(virDomainFSModel) VIR_ENUM_DECL(virDomainFSDriver) VIR_ENUM_DECL(virDomainFSAccessMode) VIR_ENUM_DECL(virDomainFSWrpolicy) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 libvirt 0/8] Add support for qemu usb-mtp device
This series adds support for qemu usb-mtp devices. This new series addresses comments for v1: https://www.redhat.com/archives/libvir-list/2014-August/msg00381.html Now the XML looks like: Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 Giuseppe Scrivano (8): conf: add child element to conf, virDomainFSDefPtr: rename "path" argument to "target" qemu_command: fix block indentation qemu: add support for MTP filesystem docs: add documentation and schema for / docs: relax / dir element attribute to string conf: fix comment tests: add tests for filesystem model mtp docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 17 ++-- src/conf/domain_conf.c | 29 - src/conf/domain_conf.h | 15 ++- src/qemu/qemu_command.c| 74 +++--- tests/domainconfdata/getfilesystem.xml | 5 +++ tests/domainconftest.c | 1 + 7 files changed, 108 insertions(+), 39 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
"Daniel P. Berrange" writes: >> Would you agree with something like this? >> >> >> mtp >>... > > What is the name="mtp share" bit trying to reflect ? > > It seems we're mostly biased towards so I think we should aim > for it is the name of the MTP share within the guest. How will it look with ? Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
"Daniel P. Berrange" writes: > On Thu, Aug 07, 2014 at 04:10:31PM +0200, Giuseppe Scrivano wrote: >> Signed-off-by: Giuseppe Scrivano >> --- >> src/conf/domain_conf.c | 34 +- >> src/conf/domain_conf.h | 1 + >> 2 files changed, 26 insertions(+), 9 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index c25c74b..3bdf46a 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -342,7 +342,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, >>"file", >>"template", >>"ram", >> - "bind") >> + "bind", >> + "mtp") > > I don't think this is the right way to represent it. > > The 'type' attribute on represents where the backing store > for the filesystem comes from. > > The distinction of 9p vs mtp reflects the type of guest device to expose > it as. > > We shouldn't try to overload these two concepts in the same attribute. > We should instead try to add a or child element as we > have for some other device types. I see, thanks for the clarification. Would you agree with something like this? mtp ... Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt 4/6] qemu: add support for MTP filesystem
Generate the qemu command line option: -device 'usb-mtp,root=$SRC,desc=$TARGET' from the definition XML: Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 53 - 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 716be0a..ad5b318 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3955,12 +3955,6 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only supports mount filesystem type")); -goto error; -} - if (!driver) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Filesystem driver type not supported")); @@ -4029,22 +4023,26 @@ qemuBuildFSDevStr(virDomainDefPtr def, { virBuffer opt = VIR_BUFFER_INITIALIZER; -if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("can only passthrough directories")); -goto error; -} +if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { +virBufferAddLit(&opt, "virtio-9p-pci"); +virBufferAsprintf(&opt, ",id=%s", fs->info.alias); +virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); +virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); -virBufferAddLit(&opt, "virtio-9p-pci"); -virBufferAsprintf(&opt, ",id=%s", fs->info.alias); -virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); -virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); - -if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) -goto error; +if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) +goto error; -if (virBufferCheckError(&opt) < 0) +if (virBufferCheckError(&opt) < 0) +goto error; +} +else if (fs->type == VIR_DOMAIN_FS_TYPE_MTP) { +virBufferAddLit(&opt, "usb-mtp"); +virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src, fs->dst); +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported filesystem type")); goto error; +} return virBufferContentAndReset(&opt); @@ -8310,11 +8308,20 @@ qemuBuildCommandLine(virConnectPtr conn, char *optstr; virDomainFSDefPtr fs = def->fss[i]; -virCommandAddArg(cmd, "-fsdev"); -if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) +if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT && +fs->type != VIR_DOMAIN_FS_TYPE_MTP) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports mount or mtp filesystem type")); goto error; -virCommandAddArg(cmd, optstr); -VIR_FREE(optstr); +} + +if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { +virCommandAddArg(cmd, "-fsdev"); +if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) +goto error; +virCommandAddArg(cmd, optstr); +VIR_FREE(optstr); +} virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps))) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt 5/6] docs: add documentation and schema for MTP filesystem type
Signed-off-by: Giuseppe Scrivano --- docs/formatdomain.html.in | 9 + docs/schemas/domaincommon.rng | 15 --- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e5b1adb..938409b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2309,6 +2309,10 @@ <target dir='/import/from/host'/> <readonly/> </filesystem> +<filesystem type='mtp'> + <source dir='/export/to/guest'/> + <target name='my-vm-template'/> +</filesystem> ... </devices> ... @@ -2367,6 +2371,11 @@ A directory inside the guest will be bound to another directory inside the guest. Only used by LXC driver (since 0.9.13) +type='mtp' + + A host directory will be exposed to the guest as a MTP + device. Only used by QEMU/KVM driver + (since 1.2.8) The filesystem block has an optional attribute accessmode diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 11f0fd0..dd870ef 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1899,9 +1899,18 @@ - - - + + + + + + + + + + + + -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt 6/6] tests: add tests for MTP filesystem
Signed-off-by: Giuseppe Scrivano --- tests/domainconfdata/getfilesystem.xml | 4 tests/domainconftest.c | 2 ++ 2 files changed, 6 insertions(+) diff --git a/tests/domainconfdata/getfilesystem.xml b/tests/domainconfdata/getfilesystem.xml index 2ee78b4..efcd83b 100644 --- a/tests/domainconfdata/getfilesystem.xml +++ b/tests/domainconfdata/getfilesystem.xml @@ -21,6 +21,10 @@ + + + + diff --git a/tests/domainconftest.c b/tests/domainconftest.c index 3d6ebe1..eee3193 100644 --- a/tests/domainconftest.c +++ b/tests/domainconftest.c @@ -111,6 +111,8 @@ mymain(void) DO_TEST_GET_FS("/dev", true); DO_TEST_GET_FS("/dev/pts", false); DO_TEST_GET_FS("/doesnotexist", false); +DO_TEST_GET_FS("mtp share", true); +DO_TEST_GET_FS("mtp not existing share", false); virObjectUnref(caps); virObjectUnref(xmlopt); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt 2/6] conf, virDomainFSDefPtr: rename "path" argument to "target"
Since the target for MTP is a name and not a path, make the function more generic. Signed-off-by: Giuseppe Scrivano --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3bdf46a..e8abc1a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18782,12 +18782,12 @@ virDomainFSRemove(virDomainDefPtr def, size_t i) virDomainFSDefPtr virDomainGetFilesystemForTarget(virDomainDefPtr def, -const char *path) +const char *target) { size_t i; for (i = 0; i < def->nfss; i++) { -if (STREQ(def->fss[i]->dst, path)) +if (STREQ(def->fss[i]->dst, target)) return def->fss[i]; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a4d8a76..48968f0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2485,7 +2485,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk, int *devIdx); virDomainFSDefPtr virDomainGetFilesystemForTarget(virDomainDefPtr def, - const char *path); + const char *target); int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs); int virDomainFSIndexByName(virDomainDefPtr def, const char *name); virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt 3/6] qemu_command: fix block indentation
Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a5ff10a..716be0a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3988,13 +3988,13 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, } if (fs->wrpolicy) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { - virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("filesystem writeout not supported")); - goto error; - } +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { +virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("filesystem writeout not supported")); +goto error; +} } virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
Signed-off-by: Giuseppe Scrivano --- src/conf/domain_conf.c | 34 +- src/conf/domain_conf.h | 1 + 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c25c74b..3bdf46a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -342,7 +342,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "file", "template", "ram", - "bind") + "bind", + "mtp") VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", @@ -6404,7 +6405,8 @@ virDomainFSDefParseXML(xmlNodePtr node, xmlStrEqual(cur->name, BAD_CAST "source")) { if (def->type == VIR_DOMAIN_FS_TYPE_MOUNT || -def->type == VIR_DOMAIN_FS_TYPE_BIND) +def->type == VIR_DOMAIN_FS_TYPE_BIND || +def->type == VIR_DOMAIN_FS_TYPE_MTP) source = virXMLPropString(cur, "dir"); else if (def->type == VIR_DOMAIN_FS_TYPE_FILE) source = virXMLPropString(cur, "file"); @@ -6418,7 +6420,10 @@ virDomainFSDefParseXML(xmlNodePtr node, } } else if (!target && xmlStrEqual(cur->name, BAD_CAST "target")) { -target = virXMLPropString(cur, "dir"); +if (def->type == VIR_DOMAIN_FS_TYPE_MTP) +target = virXMLPropString(cur, "name"); +else +target = virXMLPropString(cur, "dir"); } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = true; } else if (xmlStrEqual(cur->name, BAD_CAST "driver")) { @@ -15668,9 +15673,11 @@ virDomainFSDefFormat(virBufferPtr buf, } -virBufferAsprintf(buf, - "\n", - type, accessmode); +virBufferAsprintf(buf, "type != VIR_DOMAIN_FS_TYPE_MTP) +virBufferAsprintf(buf, " accessmode='%s'", accessmode); +virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (def->fsdriver) { virBufferAsprintf(buf, "\n", def->usage / 1024); break; +case VIR_DOMAIN_FS_TYPE_MTP: +virBufferEscapeString(buf, "\n", + def->src); +break; } -virBufferEscapeString(buf, "\n", - def->dst); +if (def->type == VIR_DOMAIN_FS_TYPE_MTP) +virBufferEscapeString(buf, "\n", + def->dst); +else +virBufferEscapeString(buf, "\n", + def->dst); if (def->readonly) virBufferAddLit(buf, "\n"); -if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) +if (def->type != VIR_DOMAIN_FS_TYPE_MTP && +virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bffc0a5..a4d8a76 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -760,6 +760,7 @@ typedef enum { VIR_DOMAIN_FS_TYPE_TEMPLATE, /* Expands a OS template to a guest dir */ VIR_DOMAIN_FS_TYPE_RAM, /* Mount a RAM filesystem on a guest dir */ VIR_DOMAIN_FS_TYPE_BIND, /* Binds a guest dir to another guest dir */ +VIR_DOMAIN_FS_TYPE_MTP, /* Binds a host dir to a MTP guest device */ VIR_DOMAIN_FS_TYPE_LAST } virDomainFSType; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt 0/6] Add support for qemu usb-mtp device
This series adds support for qemu usb-mtp devices. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 Giuseppe Scrivano (6): conf: add MTP filesystem support to the parser conf, virDomainFSDefPtr: rename "path" argument to "target" qemu_command: fix block indentation qemu: add support for MTP filesystem docs: add documentation and schema for MTP filesystem type tests: add tests for MTP filesystem docs/formatdomain.html.in | 9 + docs/schemas/domaincommon.rng | 15 ++-- src/conf/domain_conf.c | 38 +-- src/conf/domain_conf.h | 3 +- src/qemu/qemu_command.c| 67 +++--- tests/domainconfdata/getfilesystem.xml | 4 ++ tests/domainconftest.c | 2 + 7 files changed, 93 insertions(+), 45 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Multiple RNG device support
Peter Krempa writes: > Qemu and kernel support multiple RNG's, so I revived my old patch to add the > support to libvirt. > > Peter Krempa (3): > qemu: cgroup: Don't use NULL path on default backed RNGs > virtio-rng: allow multiple RNG devices > test: qemu: Add tests for multiple virtio-rng devices FYI, I was curious to see how virt-manager performed with this series so I gave it a try and everything seems to work as expected there. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt] qemu: raise an eror when using aio=native without cache=none
Qemu will fallback to aio=threads when the cache mode doesn't use O_DIRECT, even if aio=native was explictly set. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1086704 Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb64cda..8c12cad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3477,6 +3477,16 @@ qemuBuildDriveStr(virConnectPtr conn, mode = qemuDiskCacheV1TypeToString(disk->cachemode); } +if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && +disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && +disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("native I/O needs either no disk cache " + "or directsync cache mode, QEMU will fallback " + "to aio=threads")); +goto error; +} + virBufferAsprintf(&opt, ",cache=%s", mode); } else if (disk->shared && !disk->readonly) { virBufferAddLit(&opt, ",cache=off"); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt] qemu: show a warning when using aio=native without cache=none
"Daniel P. Berrange" writes: > On Tue, Jul 08, 2014 at 02:08:38PM +0200, Giuseppe Scrivano wrote: >> Qemu will fallback to aio=threads when the cache mode doesn't use >> O_DIRECT, even if aio=native was explictly set. >> >> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1086704 >> >> Signed-off-by: Giuseppe Scrivano >> --- >> src/qemu/qemu_command.c | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index fb64cda..92a6c9a 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -3477,6 +3477,13 @@ qemuBuildDriveStr(virConnectPtr conn, >> mode = qemuDiskCacheV1TypeToString(disk->cachemode); >> } >> >> +if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && >> +disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC) { >> +VIR_WARN("native I/O needs either no disk cache " >> + "or directsync cache mode, QEMU will fallback " >> + "to aio=threads"); >> +} > > VIR_WARN is not really appropriate for warning about things that > are client application mistakes. If this combination of options > does not work or is not supported then we should report an > error about this. I didn't want to block the VM execution as anyway qemu allows this combination. I will change it to use virReportError. Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt] qemu: show a warning when using aio=native without cache=none
Qemu will fallback to aio=threads when the cache mode doesn't use O_DIRECT, even if aio=native was explictly set. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1086704 Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb64cda..92a6c9a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3477,6 +3477,13 @@ qemuBuildDriveStr(virConnectPtr conn, mode = qemuDiskCacheV1TypeToString(disk->cachemode); } +if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && +disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC) { +VIR_WARN("native I/O needs either no disk cache " + "or directsync cache mode, QEMU will fallback " + "to aio=threads"); +} + virBufferAsprintf(&opt, ",cache=%s", mode); } else if (disk->shared && !disk->readonly) { virBufferAddLit(&opt, ",cache=off"); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks
Martin Kletzander writes: > If the user updates from QEMU without DRIVE_READONLY to newer one, > that supports that flag, than XML with readonly IDE HDD will stop > working even though it worked before the update *as requested*. That > readonly flag does not reflect how the disk is exposed in the guest; > as you said IDE does not have any readonly concept, that is only how > the device reacts to writes. > > Changing the behaviour to: > > if (disk->readonly && >virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY) && >!(disk->bus == VIR_DOMAIN_DISK_BUS_IDE && > disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)) >virBufferAddLit(&opt, ",readonly=on"); > > > would keep the backward compatibility. This behaviour makes more > sense to me. this behaves in a quite different way that my proposed patch but if affects also the SELinux label and we allow the process to run anyway by skipping readonly=on for IDE disks, wouldn't qemu fail in weird ways when trying to write to the file? Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks
Martin Kletzander writes: Q> On Mon, Jun 30, 2014 at 12:05:06PM +0200, Giuseppe Scrivano wrote: >>The IDE bus doesn't support readonly disks, so inform the user with an >>error message instead of let qemu fail with a more obscure "Device >>'ide-hd' could not be initialized" error message. >> >>Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939 >> >>Signed-off-by: Giuseppe Scrivano >>--- >> src/qemu/qemu_command.c | 9 - >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >>index 63f322a..4829176 100644 >>--- a/src/qemu/qemu_command.c >>+++ b/src/qemu/qemu_command.c >>@@ -3385,8 +3385,15 @@ qemuBuildDriveStr(virConnectPtr conn, >> disk->bus != VIR_DOMAIN_DISK_BUS_IDE) >> virBufferAddLit(&opt, ",boot=on"); >> if (disk->readonly && >>-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) >>+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) { >>+if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE && >>+disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { >>+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >>+ _("readonly ide disks are not supported")); >>+goto error; >>+} >> virBufferAddLit(&opt, ",readonly=on"); >>+} > > I'm not very sure we should do that. Second opinion would be great. > My point is that if qemu does not support the readonly flag, we just > skip setting it, but if it supports it, we will error out? OTOH > skipping the readonly=on when user requests it is, ehm, not nice > either. I think Eric was saying that *some* readonly flags do not > make much of a sense, but that was probably WRT backing chains or > something like that. Eric? I don't think that this error is related to the presence of the DRIVE_READONLY Qemu caps. My understanding of this comment on the Qemu bug conterpart (https://bugzilla.redhat.com/show_bug.cgi?id=915162#c1) is that this is not going to change as readonly disks are not supported by the IDE bus, and this can be considered just an early detection of this situation. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks
The IDE bus doesn't support readonly disks, so inform the user with an error message instead of let qemu fail with a more obscure "Device 'ide-hd' could not be initialized" error message. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939 Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_command.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 63f322a..4829176 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3385,8 +3385,15 @@ qemuBuildDriveStr(virConnectPtr conn, disk->bus != VIR_DOMAIN_DISK_BUS_IDE) virBufferAddLit(&opt, ",boot=on"); if (disk->readonly && -virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) +virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) { +if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE && +disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("readonly ide disks are not supported")); +goto error; +} virBufferAddLit(&opt, ",readonly=on"); +} if (disk->transient) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("transient disks not supported yet")); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] virtportallocator: new function "virPortAllocatorSetUsed"
Michal Privoznik writes: > On 24.06.2014 13:34, Giuseppe Scrivano wrote: >> virPortAllocatorSetUsed permits to set a port as already used and >> prevent the port allocator to use it without any attempt to bind it. >> >> Signed-off-by: Giuseppe Scrivano >> --- >> src/libvirt_private.syms| 1 + >> src/util/virportallocator.c | 44 >> +++- >> src/util/virportallocator.h | 4 >> 3 files changed, 48 insertions(+), 1 deletion(-) >> >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index 2a2b9c0..95a45aa 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -1770,6 +1770,7 @@ virPidFileWritePath; >> virPortAllocatorAcquire; >> virPortAllocatorNew; >> virPortAllocatorRelease; >> +virPortAllocatorSetUsed; >> >> >> # util/virprocess.h >> diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c >> index b68133a..226f386 100644 >> --- a/src/util/virportallocator.c >> +++ b/src/util/virportallocator.c >> @@ -1,7 +1,7 @@ >> /* >>* virportallocator.c: Allocate & track TCP port allocations >>* >> - * Copyright (C) 2013 Red Hat, Inc. >> + * Copyright (C) 2013, 2014 Red Hat, Inc. > > How about 2013-2014? >> +else { > I've no write access to the repo. Could these fixes be amended before pushing the patches or shall I go for v3? Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] graphics: remember graphics not auto allocated ports
When looking for a port to allocate, the port allocator didn't take in consideration ports that are statically set by the user. Defining these two graphics elements in the XML would cause an error, as the port allocator would try to use the same port for the spice graphics element: The new *[pP]ortReserved variables keep track of the ports that were successfully tracked as used by the port allocator but that weren't bound. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 Signed-off-by: Giuseppe Scrivano --- src/conf/domain_conf.h | 3 +++ src/qemu/qemu_process.c | 68 - 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6779a41..1122eb2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1385,6 +1385,7 @@ struct _virDomainGraphicsDef { union { struct { int port; +bool portReserved; int websocket; bool autoport; char *keymap; @@ -1410,6 +1411,8 @@ struct _virDomainGraphicsDef { struct { int port; int tlsPort; +bool portReserved; +bool tlsPortReserved; int mousemode; char *keymap; virDomainGraphicsAuthDef auth; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0b8155b..561e37e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3803,6 +3803,41 @@ int qemuProcessStart(virConnectPtr conn, for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; +if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && +!graphics->data.vnc.autoport) { +if (virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.vnc.port, +true) < 0) { +goto cleanup; +} + +graphics->data.vnc.portReserved = true; + +} else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + !graphics->data.spice.autoport) { + +if (graphics->data.spice.port > 0) { +if (virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.spice.port, +true) < 0) +goto cleanup; + +graphics->data.spice.portReserved = true; +} + +if (graphics->data.spice.tlsPort > 0) { +if (virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.spice.tlsPort, +true) < 0) +goto cleanup; + +graphics->data.spice.tlsPortReserved = true; +} +} +} + +for (i = 0; i < vm->def->ngraphics; ++i) { +virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { if (qemuProcessVNCAllocatePorts(driver, graphics) < 0) goto cleanup; @@ -4513,15 +4548,36 @@ void qemuProcessStop(virQEMUDriverPtr driver, virPortAllocatorRelease(driver->remotePorts, graphics->data.vnc.port); } +else if (graphics->data.vnc.portReserved) { +virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.spice.port, +false); +graphics->data.vnc.portReserved = false; +} virPortAllocatorRelease(driver->webSocketPorts, graphics->data.vnc.websocket); } -if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && -graphics->data.spice.autoport) { -virPortAllocatorRelease(driver->remotePorts, -graphics->data.spice.port); -virPortAllocatorRelease(driver->remotePorts, -graphics->data.spice.tlsPort); +if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { +if (graphics->data.spice.autoport) { +virPortAllocatorRelease(driver->remotePorts, +graphics->data.spice.port); +virPortAllocatorRelease(driver->remotePorts, +graphics->data.spice.tlsPort); +} else { +if (graphics->data.spice.portReserved) { +virPortAllocatorSetUsed(driver->remo
[libvirt] [PATCH v2 1/2] virtportallocator: new function "virPortAllocatorSetUsed"
virPortAllocatorSetUsed permits to set a port as already used and prevent the port allocator to use it without any attempt to bind it. Signed-off-by: Giuseppe Scrivano --- src/libvirt_private.syms| 1 + src/util/virportallocator.c | 44 +++- src/util/virportallocator.h | 4 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2a2b9c0..95a45aa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1770,6 +1770,7 @@ virPidFileWritePath; virPortAllocatorAcquire; virPortAllocatorNew; virPortAllocatorRelease; +virPortAllocatorSetUsed; # util/virprocess.h diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index b68133a..226f386 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -1,7 +1,7 @@ /* * virportallocator.c: Allocate & track TCP port allocations * - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013, 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 Public @@ -250,3 +250,45 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa, virObjectUnlock(pa); return ret; } + +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, +unsigned short port, +bool value) +{ +int ret = -1; + +virObjectLock(pa); + +if (port < pa->start || +port > pa->end) { +ret = 0; +goto cleanup; +} + +if (value) { +bool used = false; +if (virBitmapGetBit(pa->bitmap, port - pa->start, &used) < 0) +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to query port %d"), port); + +if (used || virBitmapSetBit(pa->bitmap, port - pa->start) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reserve port %d"), port); +goto cleanup; +} +} +else { +if (virBitmapClearBit(pa->bitmap, + port - pa->start) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to release port %d"), + port); +goto cleanup; +} +} + +ret = 0; + cleanup: +virObjectUnlock(pa); +return ret; +} diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h index c8aa6de..e5ee56d 100644 --- a/src/util/virportallocator.h +++ b/src/util/virportallocator.h @@ -38,4 +38,8 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, int virPortAllocatorRelease(virPortAllocatorPtr pa, unsigned short port); +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, +unsigned short port, +bool value); + #endif /* __VIR_PORT_ALLOCATOR_H__ */ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] virtportallocator: remember not auto allocated graphics ports
Changes in v2: * virPortAllocatorSetUsed returns an error if the port is already used. * Changed *[pP]ortAllocated to *[pP]ortReserved that keep track only of ports that were marked used but not bound. * Handle separately port and tlsPort for SPICE. Fix a conflict when both autoport graphics and statically configured ports are used, like: More details in 2/2 Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 Giuseppe Scrivano (2): virtportallocator: new function "virPortAllocatorSetUsed" graphics: remember graphics not auto allocated ports src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms| 1 + src/qemu/qemu_process.c | 68 + src/util/virportallocator.c | 44 - src/util/virportallocator.h | 4 +++ 5 files changed, 113 insertions(+), 7 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] graphics: remember graphics not auto allocated ports
Ján Tomko writes: > On 06/23/2014 08:15 PM, Giuseppe Scrivano wrote: >> When looking for a port to allocate, the port allocator didn't take in >> consideration ports that are statically set by the user. Defining >> these two graphics elements in the XML would cause an error, as the >> port allocator would try to use the same port for the spice graphics >> element: >> >> >> >> >> The new *[pP]ortAllocated variables keep track of the ports that were >> successfully registered (either bound or simply tracked as used) by >> the port allocator to allow a clean rollback on errors. >> >> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 >> >> Signed-off-by: Giuseppe Scrivano >> --- >> src/conf/domain_conf.h | 3 ++ >> src/qemu/qemu_process.c | 79 >> +++-- >> 2 files changed, 73 insertions(+), 9 deletions(-) >> > >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index 0b8155b..06f1e54 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -3487,6 +3487,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, >> if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) >> return -1; >> graphics->data.vnc.port = port; >> +graphics->data.vnc.portAllocated = true; >> } >> >> if (graphics->data.vnc.websocket == -1) { >> @@ -3548,6 +3549,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, >> goto error; >> >> graphics->data.spice.port = port; >> +graphics->data.spice.portAllocated = true; >> } >> >> if (needTLSPort || graphics->data.spice.tlsPort == -1) { >> @@ -3573,6 +3575,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, >> goto error; >> >> graphics->data.spice.tlsPort = tlsPort; >> +graphics->data.spice.tlsPortAllocated = true; >> } >> } >> >> @@ -3803,6 +3806,36 @@ int qemuProcessStart(virConnectPtr conn, >> >> for (i = 0; i < vm->def->ngraphics; ++i) { >> virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; >> +if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && >> +!graphics->data.vnc.autoport) { >> +if (virPortAllocatorSetUsed(driver->remotePorts, >> +graphics->data.vnc.port, >> +true) < 0) { > > virPortAllocatorSetUsed doesn't error out if the static port is already marked > as used. If it's already used by another domain, this one fails to start and > releases the port in qemuProcessStop. > > If you change it to fail on occupied ports, it will also catch duplicit static > ports, but only in the port allocator range. I expect someone will file a bug > about conflicting ports out of that range as well :) sorry, it had to be failing on occupied ports, I'll change it in v2. > >> +goto cleanup; >> +} >> + >> +graphics->data.vnc.portAllocated = true; >> + >> +} else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && >> + !graphics->data.spice.autoport) { > > If either of the ports is -1, you shouldn't be reserving them. (For VNC, we > set autoport to true if port is -1 when parsing the XML. For SPICE, this only > happens if both ports are -1). I will modify it to reserve a port only if !autoport && port != -1 and fix the release part too. Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] graphics: remember graphics not auto allocated ports
When looking for a port to allocate, the port allocator didn't take in consideration ports that are statically set by the user. Defining these two graphics elements in the XML would cause an error, as the port allocator would try to use the same port for the spice graphics element: The new *[pP]ortAllocated variables keep track of the ports that were successfully registered (either bound or simply tracked as used) by the port allocator to allow a clean rollback on errors. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 Signed-off-by: Giuseppe Scrivano --- src/conf/domain_conf.h | 3 ++ src/qemu/qemu_process.c | 79 +++-- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6779a41..7617574 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1385,6 +1385,7 @@ struct _virDomainGraphicsDef { union { struct { int port; +bool portAllocated; int websocket; bool autoport; char *keymap; @@ -1410,6 +1411,8 @@ struct _virDomainGraphicsDef { struct { int port; int tlsPort; +bool portAllocated; +bool tlsPortAllocated; int mousemode; char *keymap; virDomainGraphicsAuthDef auth; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0b8155b..06f1e54 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3487,6 +3487,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) return -1; graphics->data.vnc.port = port; +graphics->data.vnc.portAllocated = true; } if (graphics->data.vnc.websocket == -1) { @@ -3548,6 +3549,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, goto error; graphics->data.spice.port = port; +graphics->data.spice.portAllocated = true; } if (needTLSPort || graphics->data.spice.tlsPort == -1) { @@ -3573,6 +3575,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, goto error; graphics->data.spice.tlsPort = tlsPort; +graphics->data.spice.tlsPortAllocated = true; } } @@ -3803,6 +3806,36 @@ int qemuProcessStart(virConnectPtr conn, for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; +if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && +!graphics->data.vnc.autoport) { +if (virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.vnc.port, +true) < 0) { +goto cleanup; +} + +graphics->data.vnc.portAllocated = true; + +} else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + !graphics->data.spice.autoport) { +if (virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.spice.port, +true) < 0) +goto cleanup; + +graphics->data.spice.portAllocated = true; + +if (virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.spice.tlsPort, +true) < 0) +goto cleanup; + +graphics->data.spice.tlsPortAllocated = true; +} +} + +for (i = 0; i < vm->def->ngraphics; ++i) { +virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { if (qemuProcessVNCAllocatePorts(driver, graphics) < 0) goto cleanup; @@ -4509,19 +4542,47 @@ void qemuProcessStop(virQEMUDriverPtr driver, 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); +if (graphics->data.vnc.portAllocated) { +if (graphics->data.vnc.autoport) { +virPortAllocatorRelease(driver->remotePorts, +graphics->data.vnc.port); +} else { +virPortAllocatorSetUsed(driver->remotePorts, +graphics->data.vnc.port, +
[libvirt] [PATCH 0/2] virtportallocator: remember not auto allocated graphics ports
Fix a conflict when both autoport graphics and statically configured ports are used, like: More details in 2/2 Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 Giuseppe Scrivano (2): virtportallocator: new function "virPortAllocatorSetUsed" graphics: remember graphics not auto allocated ports src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms| 1 + src/qemu/qemu_process.c | 79 +++-- src/util/virportallocator.c | 40 ++- src/util/virportallocator.h | 4 +++ 5 files changed, 117 insertions(+), 10 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virtportallocator: new function "virPortAllocatorSetUsed"
virPortAllocatorSetUsed permits to set a port as already used and prevent the port allocator to use it without any attempt to bind it. Signed-off-by: Giuseppe Scrivano --- src/libvirt_private.syms| 1 + src/util/virportallocator.c | 40 +++- src/util/virportallocator.h | 4 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2a2b9c0..95a45aa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1770,6 +1770,7 @@ virPidFileWritePath; virPortAllocatorAcquire; virPortAllocatorNew; virPortAllocatorRelease; +virPortAllocatorSetUsed; # util/virprocess.h diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index b68133a..e98fc8a 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -1,7 +1,7 @@ /* * virportallocator.c: Allocate & track TCP port allocations * - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013, 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 Public @@ -250,3 +250,41 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa, virObjectUnlock(pa); return ret; } + +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, +unsigned short port, +bool used) +{ +int ret = -1; + +virObjectLock(pa); + + +if (port < pa->start || +port > pa->end) { +ret = 0; +goto cleanup; +} + +if (used) { +if (virBitmapSetBit(pa->bitmap, port - pa->start) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reserve port %d"), port); +goto cleanup; +} +} +else { +if (virBitmapClearBit(pa->bitmap, + port - pa->start) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to release port %d"), + port); +goto cleanup; +} +} + +ret = 0; + cleanup: +virObjectUnlock(pa); +return ret; +} diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h index c8aa6de..e5ee56d 100644 --- a/src/util/virportallocator.h +++ b/src/util/virportallocator.h @@ -38,4 +38,8 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, int virPortAllocatorRelease(virPortAllocatorPtr pa, unsigned short port); +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, +unsigned short port, +bool value); + #endif /* __VIR_PORT_ALLOCATOR_H__ */ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: report VIR_ERR_NO_STORAGE_VOL when the file doesn't exist
Report VIR_ERR_NO_STORAGE_VOL instead of a system error when lstat fails because the file doesn't exist. Fixes this problem in virt-install: https://bugzilla.redhat.com/show_bug.cgi?id=1108922 Signed-off-by: Giuseppe Scrivano --- src/storage/storage_backend.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 380ca58..b38af52 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1305,9 +1305,15 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, bool noerror = (flags & VIR_STORAGE_VOL_OPEN_NOERROR); if (lstat(path, sb) < 0) { -if (errno == ENOENT && noerror) { -VIR_WARN("ignoring missing file '%s'", path); -return -2; +if (errno == ENOENT) { +if (noerror) { +VIR_WARN("ignoring missing file '%s'", path); +return -2; +} +virReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol with matching path '%s'"), + path); +return -1; } virReportSystemError(errno, _("cannot stat file '%s'"), -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCH] udev: consider the device a CDROM when ID_CDROM=1
"Daniel P. Berrange" writes: > On Wed, Apr 23, 2014 at 12:42:01PM +0200, Giuseppe Scrivano wrote: >> Some CDROM devices are reported by udev to have an ID_TYPE="generic" >> thus it is necessary to check if ID_CDROM is present. >> >> As a side effect, treating ID_TYPE="generic" as a missing ID_TYPE will >> enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the >> udevKludgeStorageType heuristic. >> >> Signed-off-by: Giuseppe Scrivano >> --- >> src/node_device/node_device_udev.c | 11 +-- >> 1 file changed, 9 insertions(+), 2 deletions(-) > > ACK Thanks. Can somebody please push the patch as I've no write access to the repo? Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCH] udev: consider the device a CDROM when ID_CDROM=1
Giuseppe Scrivano writes: > Some CDROM devices are reported by udev to have an ID_TYPE="generic" > thus it is necessary to check if ID_CDROM is present. > > As a side effect, treating ID_TYPE="generic" as a missing ID_TYPE will > enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the > udevKludgeStorageType heuristic. > > Signed-off-by: Giuseppe Scrivano ping? Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt PATCH] udev: consider the device a CDROM when ID_CDROM=1
Some CDROM devices are reported by udev to have an ID_TYPE="generic" thus it is necessary to check if ID_CDROM is present. As a side effect, treating ID_TYPE="generic" as a missing ID_TYPE will enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the udevKludgeStorageType heuristic. Signed-off-by: Giuseppe Scrivano --- src/node_device/node_device_udev.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 71b3c79..de1cc67 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1,7 +1,7 @@ /* * node_device_udev.c: node device enumeration - libudev implementation * - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-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 Public @@ -1093,7 +1093,8 @@ static int udevProcessStorage(struct udev_device *device, if (udevGetStringProperty(device, "ID_TYPE", - &data->storage.drive_type) != PROPERTY_FOUND) { + &data->storage.drive_type) != PROPERTY_FOUND +|| STREQ(def->caps->data.storage.drive_type, "generic")) { int tmp_int = 0; /* All floppy drives have the ID_DRIVE_FLOPPY prop. This is @@ -1104,6 +1105,12 @@ static int udevProcessStorage(struct udev_device *device, if (VIR_STRDUP(data->storage.drive_type, "floppy") < 0) goto out; +} else if ((udevGetIntProperty(device, "ID_CDROM", +&tmp_int, 0) == PROPERTY_FOUND) && +(tmp_int == 1)) { + +if (VIR_STRDUP(data->storage.drive_type, "cd") < 0) +goto out; } else if ((udevGetIntProperty(device, "ID_DRIVE_FLASH_SD", &tmp_int, 0) == PROPERTY_FOUND) && (tmp_int == 1)) { -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] python: add error checking for calls to PyList_New
Eric Blake writes: > But the general observation remains - we have a lot of crufty code that > does error handling incorrectly in our python bindings :) And there's > still the question of whether we are going to split python bindings into > their own repository; it might be nice to have the code cleaned up > before that point. I forgot to add that I have deliberately left the function "libvirt_virConnectDomainEventGraphicsCallback" unchanged in my patch (probably this should go in the commit message), that I prefer to address later. It requires a bigger refactoring; from what I can see, it expects things just to work. Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] python: prefer PyList_SET_ITEM to PyList_SetItem
The PyList_SET_ITEM macro, differently from PyList_SetItem, doesn't do any error checking and overwrites anything that was previously stored in the list at the chosen destination position. PyList_SET_ITEM is usually faster than PyList_SetItem, and since it is used only to fill freshly created lists, it is safe to be used here. Signed-off-by: Giuseppe Scrivano --- python/libvirt-override.c | 197 +- 1 file changed, 90 insertions(+), 107 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 2e58bf9..83bca94 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1588,8 +1588,7 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyTuple_SetItem(info, 3, item) < 0) goto itemError; -if (PyList_SetItem(pycpuinfo, i, info) < 0) -goto itemError; +PyList_SET_ITEM(pycpuinfo, i, info); continue; itemError: @@ -1611,10 +1610,7 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; } } -if (PyList_SetItem(pycpumap, i, info) < 0) { -Py_DECREF(info); -goto cleanup; -} +PyList_SET_ITEM(pycpumap, i, info); } if (PyTuple_SetItem(pyretval, 0, pycpuinfo) < 0 || PyTuple_SetItem(pyretval, 1, pycpumap) < 0) @@ -1812,7 +1808,7 @@ libvirt_virDomainGetVcpuPinInfo(PyObject *self ATTRIBUTE_UNUSED, PyTuple_SetItem(mapinfo, pcpu, PyBool_FromLong(VIR_CPU_USABLE(cpumaps, cpumaplen, vcpu, pcpu))); } -PyList_SetItem(pycpumaps, vcpu, mapinfo); +PyList_SET_ITEM(pycpumaps, vcpu, mapinfo); } VIR_FREE(cpumaps); @@ -2115,21 +2111,21 @@ static int virConnectCredCallbackWrapper(virConnectCredentialPtr cred, pycreditem = PyList_New(5); Py_INCREF(Py_None); PyTuple_SetItem(pycred, i, pycreditem); -PyList_SetItem(pycreditem, 0, PyInt_FromLong((long) cred[i].type)); -PyList_SetItem(pycreditem, 1, PyString_FromString(cred[i].prompt)); +PyList_SET_ITEM(pycreditem, 0, PyInt_FromLong((long) cred[i].type)); +PyList_SET_ITEM(pycreditem, 1, PyString_FromString(cred[i].prompt)); if (cred[i].challenge) { -PyList_SetItem(pycreditem, 2, PyString_FromString(cred[i].challenge)); +PyList_SET_ITEM(pycreditem, 2, PyString_FromString(cred[i].challenge)); } else { Py_INCREF(Py_None); -PyList_SetItem(pycreditem, 2, Py_None); +PyList_SET_ITEM(pycreditem, 2, Py_None); } if (cred[i].defresult) { -PyList_SetItem(pycreditem, 3, PyString_FromString(cred[i].defresult)); +PyList_SET_ITEM(pycreditem, 3, PyString_FromString(cred[i].defresult)); } else { Py_INCREF(Py_None); -PyList_SetItem(pycreditem, 3, Py_None); +PyList_SET_ITEM(pycreditem, 3, Py_None); } -PyList_SetItem(pycreditem, 4, Py_None); +PyList_SET_ITEM(pycreditem, 4, Py_None); } PyTuple_SetItem(list, 0, pycred); @@ -2390,7 +2386,7 @@ libvirt_virConnectListDomainsID(PyObject *self ATTRIBUTE_UNUSED, if (ids) { for (i = 0; i < c_retval; i++) { -PyList_SetItem(py_retval, i, libvirt_intWrap(ids[i])); +PyList_SET_ITEM(py_retval, i, libvirt_intWrap(ids[i])); } VIR_FREE(ids); } @@ -2426,13 +2422,12 @@ libvirt_virConnectListAllDomains(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; for (i = 0; i < c_retval; i++) { -if (!(tmp = libvirt_virDomainPtrWrap(doms[i])) || -PyList_SetItem(py_retval, i, tmp) < 0) { -Py_XDECREF(tmp); +if (!(tmp = libvirt_virDomainPtrWrap(doms[i]))) { Py_DECREF(py_retval); py_retval = NULL; goto cleanup; } +PyList_SET_ITEM(py_retval, i, tmp); /* python steals the pointer */ doms[i] = NULL; } @@ -2481,7 +2476,7 @@ libvirt_virConnectListDefinedDomains(PyObject *self ATTRIBUTE_UNUSED, if (names) { for (i = 0; i < c_retval; i++) { -PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i])); +PyList_SET_ITEM(py_retval, i, libvirt_constcharPtrWrap(names[i])); VIR_FREE(names[i]); } VIR_FREE(names); @@ -2530,13 +2525,12 @@ libvirt_virDomainSnapshotListNames(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; for (i = 0; i < c_retval; i++) { -if ((pyobj_snap = libvirt_constcharPtrWrap(names[i])) == NULL || -PyList_SetItem(py_retval, i, pyobj_snap) < 0) { -Py_XDECREF(pyobj_snap); +if ((pyobj_snap = libvirt_constcharPtrWrap(names[i])) == NULL) { Py_DECREF(py_retval); py_re
[libvirt] [PATCH 0/2] libvirt-override.c: Use PyList_SET_ITEM + error checking for PyList_New
Giuseppe Scrivano (2): python: prefer PyList_SET_ITEM to PyList_SetItem python: add error checking for calls to PyList_New python/libvirt-override.c | 285 ++ 1 file changed, 160 insertions(+), 125 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] python: add error checking for calls to PyList_New
Signed-off-by: Giuseppe Scrivano --- python/libvirt-override.c | 88 +-- 1 file changed, 70 insertions(+), 18 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 83bca94..3b902bc 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2095,7 +2095,7 @@ static int virConnectCredCallbackWrapper(virConnectCredentialPtr cred, PyObject *pyauth = (PyObject *)cbdata; PyObject *pycbdata; PyObject *pycb; -PyObject *pyret; +PyObject *pyret = NULL; int ret = -1; size_t i; @@ -2108,7 +2108,8 @@ static int virConnectCredCallbackWrapper(virConnectCredentialPtr cred, pycred = PyTuple_New(ncred); for (i = 0; i < ncred; i++) { PyObject *pycreditem; -pycreditem = PyList_New(5); +if (!(pycreditem = PyList_New(5))) +goto cleanup; Py_INCREF(Py_None); PyTuple_SetItem(pycred, i, pycreditem); PyList_SET_ITEM(pycreditem, 0, PyInt_FromLong((long) cred[i].type)); @@ -2382,7 +2383,10 @@ libvirt_virConnectListDomainsID(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_NONE; } } -py_retval = PyList_New(c_retval); +if (!(py_retval = PyList_New(c_retval))) { +VIR_FREE(ids); +return VIR_PY_NONE; +} if (ids) { for (i = 0; i < c_retval; i++) { @@ -2472,7 +2476,12 @@ libvirt_virConnectListDefinedDomains(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_NONE; } } -py_retval = PyList_New(c_retval); +if (!(py_retval = PyList_New(c_retval))) { +for (i = 0; i < c_retval; i++) +VIR_FREE(names[i]); +VIR_FREE(names); +return VIR_PY_NONE; +} if (names) { for (i = 0; i < c_retval; i++) { @@ -2621,7 +2630,8 @@ libvirt_virDomainSnapshotListChildrenNames(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_NONE; } } -py_retval = PyList_New(c_retval); +if (!(py_retval = PyList_New(c_retval))) +goto cleanup; for (i = 0; i < c_retval; i++) { if ((pyobj_snap = libvirt_constcharPtrWrap(names[i])) == NULL) { @@ -2724,7 +2734,8 @@ libvirt_virDomainGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { LIBVIRT_END_ALLOW_THREADS; if (c_retval < 0) return VIR_PY_NONE; -py_retval = PyList_New(5); +if (!(py_retval = PyList_New(5))) +return VIR_PY_NONE; PyList_SET_ITEM(py_retval, 0, libvirt_intWrap((int) info.state)); PyList_SET_ITEM(py_retval, 1, libvirt_ulongWrap(info.maxMem)); PyList_SET_ITEM(py_retval, 2, libvirt_ulongWrap(info.memory)); @@ -2757,7 +2768,9 @@ libvirt_virDomainGetState(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) if (c_retval < 0) return VIR_PY_NONE; -py_retval = PyList_New(2); +if (!(py_retval = PyList_New(2))) +return VIR_PY_NONE; + PyList_SET_ITEM(py_retval, 0, libvirt_intWrap(state)); PyList_SET_ITEM(py_retval, 1, libvirt_intWrap(reason)); return py_retval; @@ -2782,7 +2795,8 @@ libvirt_virDomainGetControlInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) LIBVIRT_END_ALLOW_THREADS; if (c_retval < 0) return VIR_PY_NONE; -py_retval = PyList_New(3); +if (!(py_retval = PyList_New(3))) +return VIR_PY_NONE; PyList_SET_ITEM(py_retval, 0, libvirt_intWrap(info.state)); PyList_SET_ITEM(py_retval, 1, libvirt_intWrap(info.details)); PyList_SET_ITEM(py_retval, 2, libvirt_longlongWrap(info.stateTime)); @@ -2808,7 +2822,9 @@ libvirt_virDomainGetBlockInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { LIBVIRT_END_ALLOW_THREADS; if (c_retval < 0) return VIR_PY_NONE; -py_retval = PyList_New(3); +if (!(py_retval = PyList_New(3))) +return VIR_PY_NONE; + PyList_SET_ITEM(py_retval, 0, libvirt_ulonglongWrap(info.capacity)); PyList_SET_ITEM(py_retval, 1, libvirt_ulonglongWrap(info.allocation)); PyList_SET_ITEM(py_retval, 2, libvirt_ulonglongWrap(info.physical)); @@ -2832,7 +2848,9 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { LIBVIRT_END_ALLOW_THREADS; if (c_retval < 0) return VIR_PY_NONE; -py_retval = PyList_New(8); +if (!(py_retval = PyList_New(8))) +return VIR_PY_NONE; + PyList_SET_ITEM(py_retval, 0, libvirt_constcharPtrWrap(&info.model[0])); PyList_SET_ITEM(py_retval, 1, libvirt_longWrap((long) info.memory >> 10)); PyList_SET_ITEM(py_retval, 2, libvirt_intWrap((int) info.cpus)); @@ -2952,7 +2970,12 @@ libvirt_virConnectListNetworks(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_NONE; } } -py_retval = PyList_New(c_retval); +if (!(py_retval = PyList_New(c_retval))) { +for (i = 0; i < c_retval; i++) +VIR_FREE(names[i]); +VIR_FREE(names); +
Re: [libvirt] [PATCH] python: prefer PyList_SET_ITEM to PyList_SetItem
Giuseppe Scrivano writes: > Martin Kletzander writes: > >> Oh, you're right. I was blinded by the fact, that PyList_SetItem() >> checks whether the 'op' is a list and I was under the impression that >> PyList_Check(op) will catch that. This has not yet changed (I checked >> 3.3.2), so you're right, the fact that we'll use PyList_SET_ITEM won't >> change a thing. I'll continue with the review in a minute. Thanks >> for the idea. > If you already worked on the review, then I can split these additional > changes and rebase them. I'll send these changes as a separate series. Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] test driver: add support for .connectBaselineCPU
It uses the same functionalities of the qemu driver. Signed-off-by: Giuseppe Scrivano --- src/test/test_driver.c | 17 + 1 file changed, 17 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c2e530e..2678247 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1512,6 +1512,21 @@ static int testConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, return 32; } +static char * +testConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, + const char **xmlCPUs, + unsigned int ncpus, + unsigned int flags) +{ +char *cpu; + +virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + +cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); + +return cpu; +} + static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { @@ -7177,6 +7192,8 @@ static virDriver testDriver = { .domainSnapshotCreateXML = testDomainSnapshotCreateXML, /* 1.1.4 */ .domainRevertToSnapshot = testDomainRevertToSnapshot, /* 1.1.4 */ .domainSnapshotDelete = testDomainSnapshotDelete, /* 1.1.4 */ + +.connectBaselineCPU = testConnectBaselineCPU, /* 1.1.5 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] python: prefer PyList_SET_ITEM to PyList_SetItem
Martin Kletzander writes: > Oh, you're right. I was blinded by the fact, that PyList_SetItem() > checks whether the 'op' is a list and I was under the impression that > PyList_Check(op) will catch that. This has not yet changed (I checked > 3.3.2), so you're right, the fact that we'll use PyList_SET_ITEM won't > change a thing. I'll continue with the review in a minute. Thanks > for the idea. If you haven't done the review yet, I can send you a new version in a while that adds error checking for PyList_New. If you already worked on the review, then I can split these additional changes and rebase them. Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] python: prefer PyList_SET_ITEM to PyList_SetItem
Martin Kletzander writes: > Not that it has any connection to your patch, but I noticed that, > somewhere, we check the return value of PyList_New(), but somewhere we > don't... I guess we should do it everywhere, shouldn't we? > > ... thinking about it, I searched through the code and > PyList_SetItem() properly errors out when 'op' (its first param) is > NULL, but PyList_SET_ITEM() will just segfault. I think we should > properly handle allocation errors before optimizing it this way. Great you have noticed it. I expected PyList_SetItem to crash too when the list == NULL. Thanks for the review. Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] python: prefer PyList_SET_ITEM to PyList_SetItem
The PyList_SET_ITEM macro, differently from PyList_SetItem, doesn't do any error checking and overwrites anything that was previously stored in the list at the chosen destination position. PyList_SET_ITEM is usually faster than PyList_SetItem, and since it is used only to fill freshly created lists, it is safe to be used here. Signed-off-by: Giuseppe Scrivano --- python/libvirt-override.c | 197 +- 1 file changed, 90 insertions(+), 107 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 2e58bf9..1e1a2ee 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1588,8 +1588,7 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyTuple_SetItem(info, 3, item) < 0) goto itemError; -if (PyList_SetItem(pycpuinfo, i, info) < 0) -goto itemError; +PyList_SET_ITEM(pycpuinfo, i, info); continue; itemError: @@ -1611,10 +1610,7 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; } } -if (PyList_SetItem(pycpumap, i, info) < 0) { -Py_DECREF(info); -goto cleanup; -} +PyList_SET_ITEM(pycpumap, i, info); } if (PyTuple_SetItem(pyretval, 0, pycpuinfo) < 0 || PyTuple_SetItem(pyretval, 1, pycpumap) < 0) @@ -1812,7 +1808,7 @@ libvirt_virDomainGetVcpuPinInfo(PyObject *self ATTRIBUTE_UNUSED, PyTuple_SetItem(mapinfo, pcpu, PyBool_FromLong(VIR_CPU_USABLE(cpumaps, cpumaplen, vcpu, pcpu))); } -PyList_SetItem(pycpumaps, vcpu, mapinfo); +PyList_SET_ITEM(pycpumaps, vcpu, mapinfo); } VIR_FREE(cpumaps); @@ -2115,21 +2111,21 @@ static int virConnectCredCallbackWrapper(virConnectCredentialPtr cred, pycreditem = PyList_New(5); Py_INCREF(Py_None); PyTuple_SetItem(pycred, i, pycreditem); -PyList_SetItem(pycreditem, 0, PyInt_FromLong((long) cred[i].type)); -PyList_SetItem(pycreditem, 1, PyString_FromString(cred[i].prompt)); +PyList_SET_ITEM(pycreditem, 0, PyInt_FromLong((long) cred[i].type)); +PyList_SET_ITEM(pycreditem, 1, PyString_FromString(cred[i].prompt)); if (cred[i].challenge) { -PyList_SetItem(pycreditem, 2, PyString_FromString(cred[i].challenge)); +PyList_SET_ITEM(pycreditem, 2, PyString_FromString(cred[i].challenge)); } else { Py_INCREF(Py_None); -PyList_SetItem(pycreditem, 2, Py_None); +PyList_SET_ITEM(pycreditem, 2, Py_None); } if (cred[i].defresult) { -PyList_SetItem(pycreditem, 3, PyString_FromString(cred[i].defresult)); +PyList_SET_ITEM(pycreditem, 3, PyString_FromString(cred[i].defresult)); } else { Py_INCREF(Py_None); -PyList_SetItem(pycreditem, 3, Py_None); +PyList_SET_ITEM(pycreditem, 3, Py_None); } -PyList_SetItem(pycreditem, 4, Py_None); +PyList_SET_ITEM(pycreditem, 4, Py_None); } PyTuple_SetItem(list, 0, pycred); @@ -2390,7 +2386,7 @@ libvirt_virConnectListDomainsID(PyObject *self ATTRIBUTE_UNUSED, if (ids) { for (i = 0; i < c_retval; i++) { -PyList_SetItem(py_retval, i, libvirt_intWrap(ids[i])); +PyList_SET_ITEM(py_retval, i, libvirt_intWrap(ids[i])); } VIR_FREE(ids); } @@ -2426,13 +2422,12 @@ libvirt_virConnectListAllDomains(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; for (i = 0; i < c_retval; i++) { -if (!(tmp = libvirt_virDomainPtrWrap(doms[i])) || -PyList_SetItem(py_retval, i, tmp) < 0) { -Py_XDECREF(tmp); +if (!(tmp = libvirt_virDomainPtrWrap(doms[i]))) { Py_DECREF(py_retval); py_retval = NULL; goto cleanup; } +PyList_SET_ITEM(py_retval, i, tmp); /* python steals the pointer */ doms[i] = NULL; } @@ -2481,7 +2476,7 @@ libvirt_virConnectListDefinedDomains(PyObject *self ATTRIBUTE_UNUSED, if (names) { for (i = 0; i < c_retval; i++) { -PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i])); +PyList_SET_ITEM(py_retval, i, libvirt_constcharPtrWrap(names[i])); VIR_FREE(names[i]); } VIR_FREE(names); @@ -2530,13 +2525,12 @@ libvirt_virDomainSnapshotListNames(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; for (i = 0; i < c_retval; i++) { -if ((pyobj_snap = libvirt_constcharPtrWrap(names[i])) == NULL || -PyList_SetItem(py_retval, i, pyobj_snap) < 0) { -Py_XDECREF(pyobj_snap); +if ((pyobj_snap = libvirt_constcharPtrWrap(names[i])) == NULL) { Py_DECREF(py_retval); py_re
[libvirt] [PATCH v4 3/3] capabilities: add baselabel per sec driver/virt type to secmodel
Expand the "secmodel" XML fragment of "host" with a sequence of baselabel's which describe the default security context used by libvirt with a specific security model and virtualization type: selinux 0 system_u:system_r:svirt_t:s0 system_u:system_r:svirt_tcg_t:s0 dac 0 107:107 107:107 "baselabel" is driver-specific information, e.g. in the DAC security model, it indicates USER_ID:GROUP_ID. Signed-off-by: Giuseppe Scrivano --- docs/schemas/capability.rng | 8 src/conf/capabilities.c | 60 +++- src/conf/capabilities.h | 14 +++ src/libvirt_private.syms | 1 + src/lxc/lxc_conf.c | 10 - src/qemu/qemu_conf.c | 21 -- tests/capabilityschemadata/caps-qemu-kvm.xml | 2 + tests/capabilityschemadata/caps-test3.xml| 2 + 8 files changed, 111 insertions(+), 7 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 65c7c72..aee03d7 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -60,6 +60,14 @@ + + + + + + + + diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1acc936..1eb5e3a 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -184,6 +184,20 @@ virCapabilitiesFreeNUMAInfo(virCapsPtr caps) } static void +virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel) +{ +size_t i; +for (i = 0; i < secmodel->nlabels; i++) { +VIR_FREE(secmodel->labels[i].type); +VIR_FREE(secmodel->labels[i].label); +} + +VIR_FREE(secmodel->labels); +VIR_FREE(secmodel->model); +VIR_FREE(secmodel->doi); +} + +static void virCapabilitiesDispose(void *object) { virCapsPtr caps = object; @@ -204,8 +218,7 @@ virCapabilitiesDispose(void *object) VIR_FREE(caps->host.migrateTrans); for (i = 0; i < caps->host.nsecModels; i++) { -VIR_FREE(caps->host.secModels[i].model); -VIR_FREE(caps->host.secModels[i].doi); +virCapabilitiesClearSecModel(&caps->host.secModels[i]); } VIR_FREE(caps->host.secModels); @@ -507,6 +520,44 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, } /** + * virCapabilitiesHostSecModelAddBaseLabel + * @secmodel: Security model to add a base label for + * @type: virtualization type + * @label: base label + * + * Returns non-zero on error. + */ +extern int +virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel, +const char *type, +const char *label) +{ +char *t = NULL, *l = NULL; + +if (type == NULL || label == NULL) +return -1; + +if (VIR_STRDUP(t, type) < 0) +goto no_memory; + +if (VIR_STRDUP(l, label) < 0) +goto no_memory; + +if (VIR_EXPAND_N(secmodel->labels, secmodel->nlabels, 1) < 0) +goto no_memory; + +secmodel->labels[secmodel->nlabels - 1].type = t; +secmodel->labels[secmodel->nlabels - 1].label = l; + +return 0; + +no_memory: +VIR_FREE(l); +VIR_FREE(t); +return -1; +} + +/** * virCapabilitiesSupportsGuestArch: * @caps: capabilities to query * @arch: Architecture to search for @@ -826,6 +877,11 @@ virCapabilitiesFormatXML(virCapsPtr caps) caps->host.secModels[i].model); virBufferAsprintf(&xml, " %s\n", caps->host.secModels[i].doi); +for (j = 0; j < caps->host.secModels[i].nlabels; j++) { +virBufferAsprintf(&xml, " %s\n", + caps->host.secModels[i].labels[j].type, + caps->host.secModels[i].labels[j].label); +} virBufferAddLit(&xml, "\n"); } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 88ec454..5bc7bb5 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -104,11 +104,20 @@ struct _virCapsHostNUMACell { virCapsHostNUMACellCPUPtr cpus; }; +typedef struct _virCapsHostSecModelLabel virCapsHostSecModelLabel; +typedef virCapsHostSecModelLabel *virCapsHostSecModelLabelPtr; +struct _virCapsHostSecModelLabel { +char *type; +char *label; +}; + typedef struct _virCapsHostSecModel virCapsHostSecModel; typedef virCapsHostSecModel *virCapsHostSecModelPtr; struct _virCapsHostSecModel { char *model; char *doi; +size_t nlabels; +virCapsHostSecModelLabelPtr labels; }; typedef struct _virCapsHost virCapsHost; @@ -225,6 +234,11 @@ virCapabilitiesAd
[libvirt] [PATCH v4 2/3] security: add new internal function "virSecurityManagerGetBaseLabel"
virSecurityManagerGetBaseLabel queries the default settings used by a security model. Signed-off-by: Giuseppe Scrivano --- src/libvirt_private.syms | 1 + src/security/security_apparmor.c | 8 src/security/security_dac.c | 10 ++ src/security/security_driver.h | 4 src/security/security_manager.c | 15 +++ src/security/security_manager.h | 2 ++ src/security/security_nop.c | 10 ++ src/security/security_selinux.c | 12 src/security/security_stack.c| 9 + 9 files changed, 71 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 84c1c28..f082c04 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -842,6 +842,7 @@ virSecurityDriverLookup; # security/security_manager.h virSecurityManagerClearSocketLabel; virSecurityManagerGenLabel; +virSecurityManagerGetBaseLabel; virSecurityManagerGetDOI; virSecurityManagerGetModel; virSecurityManagerGetMountOptions; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 30e4c3f..776a470 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -931,6 +931,12 @@ AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return opts; } +static const char * +AppArmorGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + int virtType ATTRIBUTE_UNUSED) +{ +return NULL; +} virSecurityDriver virAppArmorSecurityDriver = { .privateDataLen = 0, @@ -972,4 +978,6 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityTapFDLabel= AppArmorSetFDLabel, .domainGetSecurityMountOptions = AppArmorGetMountOptions, + +.getBaseLabel = AppArmoryGetBaseLabel, }; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index f16251c..019c789 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1174,6 +1174,14 @@ virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return NULL; } +static const char * +virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr, + int virt ATTRIBUTE_UNUSED) +{ +virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); +return priv->baselabel; +} + virSecurityDriver virSecurityDriverDAC = { .privateDataLen = sizeof(virSecurityDACData), .name = SECURITY_DAC_NAME, @@ -1216,4 +1224,6 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityTapFDLabel= virSecurityDACSetTapFDLabel, .domainGetSecurityMountOptions = virSecurityDACGetMountOptions, + +.getBaseLabel = virSecurityDACGetBaseLabel, }; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 8735558..ced1b92 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -46,6 +46,8 @@ typedef int (*virSecurityDriverClose) (virSecurityManagerPtr mgr); typedef const char *(*virSecurityDriverGetModel) (virSecurityManagerPtr mgr); typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr); +typedef const char *(*virSecurityDriverGetBaseLabel) (virSecurityManagerPtr mgr, + int virtType); typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr); @@ -154,6 +156,8 @@ struct _virSecurityDriver { virSecurityDomainGetMountOptions domainGetSecurityMountOptions; virSecurityDomainSetHugepages domainSetSecurityHugepages; + +virSecurityDriverGetBaseLabel getBaseLabel; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 0e783ee..5b76ad8 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -275,6 +275,21 @@ virSecurityManagerGetModel(virSecurityManagerPtr mgr) return NULL; } +/* return NULL if a base label is not present */ +const char * +virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType) +{ +if (mgr->drv->getBaseLabel) { +const char *ret; +virObjectLock(mgr); +ret = mgr->drv->getBaseLabel(mgr, virtType); +virObjectUnlock(mgr); +return ret; +} + +return NULL; +} + bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) { return mgr->allowDiskFormatProbing; diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 9252830..81d3160 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -55,6 +55,8 @@ void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDriver(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDOI(virSecurityManagerPtr
[libvirt] [PATCH v4 1/3] security: use a single function to set DAC user and group
Merge the functions 'virSecurityDACSetUser' and 'virSecurityDACSetGroup' into 'virSecurityDACSetUserAndGroup'. Signed-off-by: Giuseppe Scrivano --- src/security/security_dac.c | 24 ++-- src/security/security_dac.h | 7 +++ src/security/security_manager.c | 6 -- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 6876bd5..f16251c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -47,22 +47,25 @@ struct _virSecurityDACData { gid_t *groups; int ngroups; bool dynamicOwnership; +char *baselabel; }; -void -virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user) +/* returns -1 on error, 0 on success */ +int +virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, + uid_t user, + gid_t group) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->user = user; -} - -void -virSecurityDACSetGroup(virSecurityManagerPtr mgr, - gid_t group) -{ -virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->group = group; + +if (virAsprintf(&priv->baselabel, "%u:%u", +(unsigned int) user, +(unsigned int) group) < 0) +return -1; + +return 0; } void @@ -217,6 +220,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); VIR_FREE(priv->groups); +VIR_FREE(priv->baselabel); return 0; } diff --git a/src/security/security_dac.h b/src/security/security_dac.h index 02432a5..dbcf56f 100644 --- a/src/security/security_dac.h +++ b/src/security/security_dac.h @@ -25,10 +25,9 @@ extern virSecurityDriver virSecurityDriverDAC; -void virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user); -void virSecurityDACSetGroup(virSecurityManagerPtr mgr, -gid_t group); +int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, + uid_t user, + gid_t group); void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, bool dynamic); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 92fb504..0e783ee 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -146,8 +146,10 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, if (!mgr) return NULL; -virSecurityDACSetUser(mgr, user); -virSecurityDACSetGroup(mgr, group); +if (virSecurityDACSetUserAndGroup(mgr, user, group) < 0) { +virSecurityManagerDispose(mgr); +return NULL; +} virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership); return mgr; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/3] expose baselabel for each sec model/virt type
Now each security model can define its own base label, that describes the default security context used by libvirt to run an hypervisor process. This information is exposed to users trough the host capabilities XML. *v4 major changes - Refactor virSecurityDACSetUser and virSecurityDACSetGroup in a separate patch - virSecurityManagerGetBaseLabel never causes a VIR_ERR_NO_SUPPORT error. *v3 major changes - support LXC - merge virSecurityDACSetUser and virSecurityDACSetGroup in virSecurityDACSetUserAndGroup - DAC sets the baselabel in virSecurityDACSetUserAndGroup - Use virDomainVirtTypeToString instead of hardcoding the name Giuseppe Scrivano (3): security: use a single function to set DAC user and group security: add new internal function "virSecurityManagerGetBaseLabel" capabilities: add baselabel per sec driver/virt type to secmodel docs/schemas/capability.rng | 8 src/conf/capabilities.c | 60 +++- src/conf/capabilities.h | 14 +++ src/libvirt_private.syms | 2 + src/lxc/lxc_conf.c | 10 - src/qemu/qemu_conf.c | 21 -- src/security/security_apparmor.c | 8 src/security/security_dac.c | 34 +++- src/security/security_dac.h | 7 ++-- src/security/security_driver.h | 4 ++ src/security/security_manager.c | 21 +- src/security/security_manager.h | 2 + src/security/security_nop.c | 10 + src/security/security_selinux.c | 12 ++ src/security/security_stack.c| 9 + tests/capabilityschemadata/caps-qemu-kvm.xml | 2 + tests/capabilityschemadata/caps-test3.xml| 2 + 17 files changed, 203 insertions(+), 23 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] expose baselabel for each sec model/virt type
Giuseppe Scrivano writes: > Now each security model can define its own base label, that describes > the default security context used by libvirt to run an hypervisor > process. This information is exposed to users trough the host > capabilities XML. > > *v3 major changes > - support LXC > - merge virSecurityDACSetUser and virSecurityDACSetGroup in > virSecurityDACSetUserAndGroup > - DAC sets the baselabel in virSecurityDACSetUserAndGroup > - Use virDomainVirtTypeToString instead of hardcoding the name I've ran a quick smoke test on top of the current HEAD and it seems to work, can someone please review it or tell me if it makes sense at all to have this information under "capabilities"? Thanks, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: use the gnulib version of the .m4 files when present
"Daniel P. Berrange" writes: > and looking at what is run by autogen.sh with strace, it seems to > use 'aclocal -I m4 -I gnulib/m4'. So this bootstrap.conf ACLOCAL > setting doesn't appear to have any effect. thanks to have checked it. It seems that using directly the "bootstrap" script makes a difference, I had assumed it doesn't; and also it makes a difference if I run "git clean -fdx" before autogen.sh (so maybe it is a separate issue?). I'll investigate this better if Eric hasn't already an answer. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: use the gnulib version of the .m4 files when present
Giuseppe Scrivano writes: > prevent aclocal to prefer .m4 files under m4/ to the version provided > by gnulib. > > I have noticed this after './configure --help' gave me two different > versions of "--enable-threads". This was caused by aclocal that > preferred the version of lock.m4 provided by autopoint instead of > using the newer version distributed with gnulib. > > Signed-off-by: Giuseppe Scrivano ping -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [virt-tools-list] [PATCH] build: fix linker error on FreeBSD
Eric Blake writes: On 10/08/2013 10:21 AM, Giuseppe Scrivano wrote: Commit 2d74822a9eb4856c7f5216bb92bcb76630660f72 renamed "freebsdNodeGetCPUCount" to "appleFreebsdNodeGetCPUCount", leaving one call to "freebsdNodeGetCPUCount". Fix this other case. ACK, although it looks like you intended this to go to libvir-list (now cc'd) I should be more careful with my bash history... What do you think about replacing completely this function with the "nproc" module in gnulib? Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: use the gnulib version of the .m4 files when present
prevent aclocal to prefer .m4 files under m4/ to the version provided by gnulib. I have noticed this after './configure --help' gave me two different versions of "--enable-threads". This was caused by aclocal that preferred the version of lock.m4 provided by autopoint instead of using the newer version distributed with gnulib. Signed-off-by: Giuseppe Scrivano --- bootstrap.conf | 5 - 1 file changed, 5 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 68c4a89..e7353e2 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -191,11 +191,6 @@ gnulib_tool_option_extras="\ " local_gl_dir=gnulib/local -# Convince bootstrap to use multiple m4 directories. -: ${ACLOCAL=aclocal} -ACLOCAL="$ACLOCAL -I m4" -export ACLOCAL - # Build prerequisites # Note that some of these programs are only required for 'make dist' to # succeed from a fresh git checkout; not all of these programs are -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: avoid compile failure on linux kernels older than 2.6.19
Signed-off-by: Giuseppe Scrivano --- tests/securityselinuxhelper.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c index 89cba3a..d996825 100644 --- a/tests/securityselinuxhelper.c +++ b/tests/securityselinuxhelper.c @@ -24,7 +24,9 @@ #include #include -#include +#if HAVE_LINUX_MAGIC_H +# include +#endif #include #include #include @@ -33,6 +35,10 @@ #include #include +#ifndef NFS_SUPER_MAGIC +# define NFS_SUPER_MAGIC 0x6969 +#endif + #include "virstring.h" static int (*realstatfs)(const char *path, struct statfs *buf); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] expose baselabel for each sec model/virt type
Giuseppe Scrivano writes: >> *v3 major changes >> - support LXC >> - merge virSecurityDACSetUser and virSecurityDACSetGroup in >> virSecurityDACSetUserAndGroup >> - DAC sets the baselabel in virSecurityDACSetUserAndGroup >> - Use virDomainVirtTypeToString instead of hardcoding the name >> >> Giuseppe Scrivano (2): >> security: add new internal function "virSecurityManagerGetBaseLabel" >> capabilities: add baselabel per sec driver/virt type to secmodel > > ping someone had a chance to look at this series? Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 7/7] python: add bindings for virConnectGetCPUModelNames
Signed-off-by: Giuseppe Scrivano --- python/generator.py | 2 +- python/libvirt-override-api.xml | 7 ++ python/libvirt-override.c | 52 + python/libvirt-override.py | 11 + 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/python/generator.py b/python/generator.py index 73107d7..12c14f1 100755 --- a/python/generator.py +++ b/python/generator.py @@ -250,6 +250,7 @@ lxc_functions_failed = [] qemu_functions_failed = [] functions_skipped = [ "virConnectListDomains", +"virConnectGetCPUModelNames", ] lxc_functions_skipped = [] qemu_functions_skipped = [] @@ -366,7 +367,6 @@ foreign_encoding_args = ( # Class methods which are written by hand in libvirt.c but the Python-level # code is still automatically generated (so they are not in skip_function()). skip_impl = ( -"virConnectGetCPUModelNames", 'virConnectGetVersion', 'virConnectGetLibVersion', 'virConnectListDomainsID', diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9a88215..1bceb05 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -476,6 +476,13 @@ + + Get the list of supported CPU models. + + + + + collect the list of snapshot names for the given domain diff --git a/python/libvirt-override.c b/python/libvirt-override.c index cc76c47..5e4acc3 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) diff --git a/python/libvirt-override.py b/python/libvirt-override.py index ccfec48..3471a43 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -207,3 +207,14 @@ def virEventAddTimeout(timeout, cb, opaque): ret = libvirtmod.virEventAddTimeout(timeout, cbData) if ret == -1: raise libvirtError ('virEventAddTimeout() failed') return ret + +def getCPUModelNames(conn, arch, flags=0): +""" +get the list of supported CPU models. +@conn: virConnect connection +@arch: Architecture +@flags: extra flags; not used yet, so callers should always pass 0. +""" +ret = libvirtmod.virConnectGetCPUModelNames(conn._o, arch, flags) +if ret == None: raise libvirtError ('virConnectGetCPUModelNames() failed', conn=self) +return ret -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 6/7] virsh: add function to get the CPU models for an arch
Signed-off-by: Giuseppe Scrivano --- tools/virsh-host.c | 54 ++ tools/virsh.pod| 5 + 2 files changed, 59 insertions(+) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index f69ab79..1d1bb97 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -665,6 +665,54 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* + * "cpu-models" command + */ +static const vshCmdInfo info_cpu_models[] = { +{.name = "help", + .data = N_("CPU models") +}, +{.name = "desc", + .data = N_("Get the CPU models for an arch.") +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_cpu_models[] = { +{.name = "arch", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("architecture") +}, +{.name = NULL} +}; + +static bool +cmdCPUModelNames(vshControl *ctl, const vshCmd *cmd) +{ +char **models; +size_t i; +int nmodels; +const char *arch = NULL; + +if (vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0) +return false; + +nmodels = virConnectGetCPUModelNames(ctl->conn, arch, &models, 0); +if (nmodels < 0) { +vshError(ctl, "%s", _("failed to get CPU model names")); +return false; +} + +for (i = 0; i < nmodels; i++) { +vshPrint(ctl, "%s\n", models[i]); +VIR_FREE(models[i]); +} +VIR_FREE(models); + +return true; +} + +/* * "version" command */ static const vshCmdInfo info_version[] = { @@ -889,6 +937,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_capabilities, .flags = 0 }, +{.name = "cpu-models", + .handler = cmdCPUModelNames, + .opts = opts_cpu_models, + .info = info_cpu_models, + .flags = 0 +}, {.name = "freecell", .handler = cmdFreecell, .opts = opts_freecell, diff --git a/tools/virsh.pod b/tools/virsh.pod index 2864f3d..e12a800 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -163,6 +163,7 @@ group as an option. For example: Host and Hypervisor (help keyword 'host'): capabilities capabilities + cpu-models show the CPU models for an architecture connect(re)connect to hypervisor freecell NUMA free memory hostname print the hypervisor hostname @@ -358,6 +359,10 @@ current domain is in. =over 4 +=item B I + +Print the list of CPU models known for the specified architecture. + =item B The domain is currently running on a CPU -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/7] virConnectGetCPUModelNames: implement the remote protocol
Signed-off-by: Giuseppe Scrivano --- daemon/remote.c | 52 src/remote/remote_driver.c | 63 src/remote/remote_protocol.x | 22 +++- src/remote_protocol-structs | 13 + 4 files changed, 149 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2ff2288..9497cc1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5056,6 +5056,58 @@ cleanup: static int +remoteDispatchConnectGetCPUModelNames(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_get_cpu_model_names_args *args, + remote_connect_get_cpu_model_names_ret *ret) +{ +int len, rv = -1; +char **models = NULL; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv->conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); +goto cleanup; +} + +len = virConnectGetCPUModelNames(priv->conn, args->arch, + args->need_results ? &models : NULL, + args->flags); +if (len < 0) +goto cleanup; + +if (len > REMOTE_CONNECT_CPU_MODELS_MAX) { +virReportError(VIR_ERR_RPC, + _("Too many CPU models '%d' for limit '%d'"), + len, REMOTE_CONNECT_CPU_MODELS_MAX); +goto cleanup; +} + +if (len && models) { +ret->models.models_val = models; +ret->models.models_len = len; +models = NULL; +} else { +ret->models.models_val = NULL; +ret->models.models_len = 0; +} + +ret->ret = len; + +rv = 0; + +cleanup: +if (rv < 0) +virNetMessageSaveError(rerr); +virStringFreeList(models); +return rv; +} + + +static int remoteDispatchDomainCreateXMLWithFiles(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 62e77a5..96ccb99 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5541,6 +5541,68 @@ done: static int +remoteConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char ***models, + unsigned int flags) +{ +int rv = -1; +size_t i; +char **retmodels = NULL; +remote_connect_get_cpu_model_names_args args; +remote_connect_get_cpu_model_names_ret ret; + +struct private_data *priv = conn->privateData; + +remoteDriverLock(priv); + +args.arch = (char *) arch; +args.need_results = !!models; +args.flags = flags; + +memset(&ret, 0, sizeof(ret)); +if (call(conn, priv, 0, REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES, + (xdrproc_t) xdr_remote_connect_get_cpu_model_names_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_get_cpu_model_names_ret, + (char *) &ret) < 0) +goto done; + +/* Check the length of the returned list carefully. */ +if (ret.models.models_len > REMOTE_CONNECT_CPU_MODELS_MAX) { +virReportError(VIR_ERR_RPC, + _("Too many model names '%d' for limit '%d'"), + ret.models.models_len, + REMOTE_CONNECT_CPU_MODELS_MAX); +goto cleanup; +} + +if (models) { +if (VIR_ALLOC_N(retmodels, ret.models.models_len + 1) < 0) +goto cleanup; + +for (i = 0; i < ret.models.models_len; i++) { +retmodels[i] = ret.models.models_val[i]; +ret.models.models_val[i] = NULL; +} +*models = retmodels; +retmodels = NULL; +} + +rv = ret.ret; + +cleanup: +virStringFreeList(retmodels); + +xdr_free((xdrproc_t) xdr_remote_connect_get_cpu_model_names_ret, (char *) &ret); + +done: +remoteDriverUnlock(priv); +return rv; +} + + +static int remoteDomainOpenGraphics(virDomainPtr dom, unsigned int idx, int fd, @@ -6933,6 +6995,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params,
[libvirt] [PATCH v4 5/7] virConnectGetCPUModelNames: add the support for the test protocol
Signed-off-by: Giuseppe Scrivano --- src/test/test_driver.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index cd65dc5..47c9d38 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -54,6 +54,7 @@ #include "virtypedparam.h" #include "virrandom.h" #include "virstring.h" +#include "cpu/cpu.h" #define VIR_FROM_THIS VIR_FROM_TEST @@ -5867,6 +5868,15 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; } +static int +testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, +const char *arch, +char ***models, +unsigned int flags) +{ +virCheckFlags(0, -1); +return cpuGetModels(arch, models); +} static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5940,6 +5950,7 @@ static virDriver testDriver = { .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ .domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */ .domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */ +.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 4/7] virConnectGetCPUModelNames: add the support for qemu
Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_driver.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8a302d1..346a8f9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15767,6 +15767,19 @@ qemuNodeSuspendForDuration(virConnectPtr conn, return nodeSuspendForDuration(target, duration, flags); } +static int +qemuConnectGetCPUModelNames(virConnectPtr conn, +const char *arch, +char ***models, +unsigned int flags) +{ +virCheckFlags(0, -1); +if (virConnectGetCPUModelNamesEnsureACL(conn) < 0) +return -1; + +return cpuGetModels(arch, models); +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -15954,6 +15967,7 @@ static virDriver qemuDriver = { .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ +.connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ }; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/7] cpu: add function to get the models for an arch
Signed-off-by: Giuseppe Scrivano --- src/cpu/cpu.c| 82 +++- src/cpu/cpu.h| 5 ++- src/libvirt_private.syms | 1 + 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 50c2de3..31de857 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -1,7 +1,7 @@ /* * cpu.c: internal functions for CPU manipulation * - * Copyright (C) 2009-2012 Red Hat, Inc. + * Copyright (C) 2009-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -27,11 +27,13 @@ #include "viralloc.h" #include "virxml.h" #include "cpu.h" +#include "cpu_map.h" #include "cpu_x86.h" #include "cpu_powerpc.h" #include "cpu_s390.h" #include "cpu_arm.h" #include "cpu_generic.h" +#include "util/virstring.h" #define NR_DRIVERS ARRAY_CARDINALITY(drivers) @@ -456,3 +458,81 @@ cpuModelIsAllowed(const char *model, } return false; } + +struct cpuGetModelsData +{ +char **data; +size_t len; /* It includes the last element of DATA, which is NULL. */ +}; + +static int +cpuGetArchModelsCb(enum cpuMapElement element, + xmlXPathContextPtr ctxt, + void *cbdata) +{ +char *name; +struct cpuGetModelsData *data = cbdata; +if (element != CPU_MAP_ELEMENT_MODEL) +return 0; + +name = virXPathString("string(@name)", ctxt); +if (name == NULL) +return -1; + +if (!data->data) { +VIR_FREE(name); +data->len++; +return 0; +} + +return VIR_INSERT_ELEMENT(data->data, data->len - 1, data->len, name); +} + + +static int +cpuGetArchModels(const char *arch, struct cpuGetModelsData *data) +{ +return cpuMapLoad(arch, cpuGetArchModelsCb, data); +} + + +int +cpuGetModels(const char *archName, char ***models) +{ +struct cpuGetModelsData data; +virArch arch; +struct cpuArchDriver *driver; +data.data = NULL; +data.len = 1; + +arch = virArchFromString(archName); +if (arch == VIR_ARCH_NONE) { +virReportError(VIR_ERR_INVALID_ARG, + _("cannot find architecture %s"), + archName); +goto error; +} + +driver = cpuGetSubDriver(arch); +if (driver == NULL) { +virReportError(VIR_ERR_INVALID_ARG, + _("cannot find a driver for the architecture %s"), + archName); +goto error; +} + +if (models && VIR_ALLOC_N(data.data, data.len) < 0) +goto error; + +if (cpuGetArchModels(driver->name, &data) < 0) +goto error; + +if (models) +*models = data.data; + +return data.len - 1; + +error: +virStringFreeList(data.data); +return -1; +} diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 69f6fae..2f0c8c7 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -1,7 +1,7 @@ /* * cpu.h: internal functions for CPU manipulation * - * Copyright (C) 2009-2010 Red Hat, Inc. + * Copyright (C) 2009-2010, 2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -175,4 +175,7 @@ cpuModelIsAllowed(const char *model, const char **models, unsigned int nmodels); +extern int +cpuGetModels(const char *arch, char ***models); + #endif /* __VIR_CPU_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 50e2f48..7665c3a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -721,6 +721,7 @@ cpuCompareXML; cpuDataFree; cpuDecode; cpuEncode; +cpuGetModels; cpuGuestData; cpuHasFeature; cpuMapOverride; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/7] cpu: add function to get the models for an arch
This series adds a new API "virConnectGetCPUModelNames" that allows to retrieve the list of CPU models known by the hypervisor for a specific architecture. This new function is mainly needed by virt-manager to not read directly the cpu_map.xml file (it could also be different when accessing a remote daemon). *v4 main changes - cpuGetModels translates the arch name to the driver name. - virConnectGetCPUModelNames handles models==NULL, and - amended all the changes reported for v3 *v3 main changes - virConnectGetCPUModelNames returns the number of models instead of 0 on success. - Use VIR_INSERT_ELEMENT instead of VIR_EXPAND_N. - Fix a potential memory leak in the python bindings. - Move virsh changes to a separate commit. - Remove API documentation from libvirt.h. *v2 main changes - set a hard limit for the number of CPU models that is possible to fetch from a remote server. - Use VIR_EXPAND_N instead of VIR_REALLOC_N. - s|1.1.2|1.1.3| Giuseppe Scrivano (7): libvirt: add new public API virConnectGetCPUModelNames cpu: add function to get the models for an arch virConnectGetCPUModelNames: implement the remote protocol virConnectGetCPUModelNames: add the support for qemu virConnectGetCPUModelNames: add the support for the test protocol virsh: add function to get the CPU models for an arch python: add bindings for virConnectGetCPUModelNames daemon/remote.c | 52 ++ include/libvirt/libvirt.h.in| 4 ++ python/generator.py | 1 + python/libvirt-override-api.xml | 7 python/libvirt-override.c | 52 ++ python/libvirt-override.py | 11 ++ src/cpu/cpu.c | 82 - src/cpu/cpu.h | 5 ++- src/driver.h| 7 src/libvirt.c | 51 + src/libvirt_private.syms| 1 + src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 14 +++ src/remote/remote_driver.c | 63 +++ src/remote/remote_protocol.x| 22 ++- src/remote_protocol-structs | 13 +++ src/test/test_driver.c | 11 ++ tools/virsh-host.c | 54 +++ tools/virsh.pod | 5 +++ 19 files changed, 457 insertions(+), 3 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/7] libvirt: add new public API virConnectGetCPUModelNames
The new function virConnectGetCPUModelNames allows to retrieve the list of CPU models known by the hypervisor for a specific architecture. Signed-off-by: Giuseppe Scrivano --- include/libvirt/libvirt.h.in | 4 python/generator.py | 1 + src/driver.h | 7 ++ src/libvirt.c| 51 src/libvirt_public.syms | 5 + 5 files changed, 68 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7944dfb..83c219e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4006,6 +4006,10 @@ int virConnectCompareCPU(virConnectPtr conn, const char *xmlDesc, unsigned int flags); +int virConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char ***models, + unsigned int flags); /** * virConnectBaselineCPUFlags diff --git a/python/generator.py b/python/generator.py index a91dde8..73107d7 100755 --- a/python/generator.py +++ b/python/generator.py @@ -366,6 +366,7 @@ foreign_encoding_args = ( # Class methods which are written by hand in libvirt.c but the Python-level # code is still automatically generated (so they are not in skip_function()). skip_impl = ( +"virConnectGetCPUModelNames", 'virConnectGetVersion', 'virConnectGetLibVersion', 'virConnectListDomainsID', diff --git a/src/driver.h b/src/driver.h index be64333..8cd164a 100644 --- a/src/driver.h +++ b/src/driver.h @@ -682,6 +682,12 @@ typedef char * unsigned int flags); typedef int +(*virDrvConnectGetCPUModelNames)(virConnectPtr conn, + const char *args, + char ***models, + unsigned int flags); + +typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1332,6 +1338,7 @@ struct _virDriver { virDrvDomainMigratePerform3Params domainMigratePerform3Params; virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; +virDrvConnectGetCPUModelNames connectGetCPUModelNames; }; diff --git a/src/libvirt.c b/src/libvirt.c index 3f65f12..6f99ed9 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18525,6 +18525,57 @@ error: /** + * virConnectGetCPUModelNames: + * + * @conn: virConnect connection + * @arch: Architecture + * @models: Pointer to a variable to store the NULL-terminated array of the + * CPU models supported for the specified architecture. Each element + * and the array itself must be freed by the caller with free. Pass + * NULL if only the list length is needed. + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the list of supported CPU models for a specific architecture. + * + * Returns -1 on error, number of elements in @models on success. + */ +int +virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, + unsigned int flags) +{ +VIR_DEBUG("conn=%p, arch=%s, models=%p, flags=%x", + conn, arch, models, flags); + virResetLastError(); + +if (models) +*models = NULL; + +if (!VIR_IS_CONNECT(conn)) { +virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} +virCheckNonNullArgReturn(arch, -1); + +if (conn->driver->connectGetCPUModelNames) { +int ret; + +ret = conn->driver->connectGetCPUModelNames(conn, arch, models, flags); +if (ret < 0) +goto error; + +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(conn); +return -1; +} + + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..fe9b497 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -634,4 +634,9 @@ LIBVIRT_1.1.1 { virDomainSetMemoryStatsPeriod; } LIBVIRT_1.1.0; +LIBVIRT_1.1.3 { +global: +virConnectGetCPUModelNames; +} LIBVIRT_1.1.1; + # define new API here using predicted next version number -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] doc: fix XML for the RNG device example
Add a missing '/' to close the "source" element. Signed-off-by: Giuseppe Scrivano --- A minor detail that I have noticed while I copy-pasting the example. 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 a927643..3689399 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4903,8 +4903,8 @@ qemu-kvm -net nic,model=? /dev/null <backend model='random'>/dev/random</backend> <!-- OR --> <backend model='egd' type='udp'> -<source mode='bind' service='1234'> -<source mode='connect' host='1.2.3.4' service='1234'> +<source mode='bind' service='1234'/> +<source mode='connect' host='1.2.3.4' service='1234'/> </backend> </rng> </devices> -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] expose baselabel for each sec model/virt type
Giuseppe Scrivano writes: > Now each security model can define its own base label, that describes > the default security context used by libvirt to run an hypervisor > process. This information is exposed to users trough the host > capabilities XML. > > *v3 major changes > - support LXC > - merge virSecurityDACSetUser and virSecurityDACSetGroup in > virSecurityDACSetUserAndGroup > - DAC sets the baselabel in virSecurityDACSetUserAndGroup > - Use virDomainVirtTypeToString instead of hardcoding the name > > Giuseppe Scrivano (2): > security: add new internal function "virSecurityManagerGetBaseLabel" > capabilities: add baselabel per sec driver/virt type to secmodel ping -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/7] cpu: add function to get the models for an arch
Eric Blake writes: > NACK, needs a v4; this is where we need to fix things to do the right > subdriver mapping. Quoting IRC: > > and I know which one to change > [15:30] jdenemareblake: cpu-models should work for both x86 and > x86_64 imho > [15:31] eblake yep - where the map file lists x86, we need the > cpu-models to support it for both i686 and x86_64 > [15:32] jdenemaryeah, the x86 in cpu_map.xml is actually a cpu > driver > name and the driver has a list of archs it supports > [15:34] eblake jdenemar: giuseppe_s used cpuMapLoad(arch, ...) - which > is only doing a literal string match > [15:34] eblake so where do we reverse map the driver names in the map > file into actual arch names? > [15:35] eblake or, where SHOULD we be doing that mapping? > [15:38] jdenemarevery cpu api in cpu.c calls cpuGetSubDriver to > get the > driver from a real arch > [15:41] jdenemarso there should be a high level cpu api that > takes a > real arch and gives model names, that should look up the appropriate sub > driver, call its api and it should load its section of cpu_map.xml and > return the list Jiri, would something like this suffice or am I missing some other details? Thanks, Giuseppe +int +cpuGetModels(const char *archName, char ***models) +{ +struct cpuGetModelsData data; +virArch arch; +struct cpuArchDriver *driver; +data.data = NULL; +data.len = 1; + +arch = virArchFromString(archName); +if (arch == VIR_ARCH_NONE) { +virReportError(VIR_ERR_INVALID_ARG, + _("cannot find architecture %s"), + archName); +goto error; +} + +driver = cpuGetSubDriver(arch); +if (driver == NULL) { +virReportError(VIR_ERR_INVALID_ARG, + _("cannot find a driver for the architecture %s"), + archName); +goto error; +} + +if (models && VIR_ALLOC_N(data.data, data.len) < 0) +goto error; + +if (cpuGetArchModels(driver->name, &data) < 0) +goto error; + +if (models) +*models = data.data; + +return data.len - 1; + +error: +virStringFreeList(data.data); +return -1; +} -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 7/7] python: add bindings for virConnectGetCPUModelNames
Eric Blake writes: > On 09/11/2013 08:13 AM, Giuseppe Scrivano wrote: >> Signed-off-by: Giuseppe Scrivano >> --- >> python/generator.py | 2 +- >> python/libvirt-override-api.xml | 7 ++ >> python/libvirt-override.c | 52 >> + >> python/libvirt-override.py | 11 + >> 4 files changed, 71 insertions(+), 1 deletion(-) >> > >> + >> +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); > > Elsewhere, we've used PyList_New(0)/PyList_Append() rather than > PyList_New(count)/PyList_SET_ITEM(); but that's not universal; also, I > see uses of PyList_SetItem but not PyList_SET_ITEM; what's the difference? PyList_SET_ITEM is just an optimized version of PyList_SetItem that doesn't do any error checking or attempt to release an object already present in the slot. It is ok to use here, as we are sure the rv list is free when we fill it with model names. I think there are other cases in libvirt-override.c where we can replace PyList_SetItem with PyList_SET_ITEM without any harm. Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 5/7] virConnectGetCPUModelNames: add the support for the test protocol
Signed-off-by: Giuseppe Scrivano --- src/test/test_driver.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f9eee44..fb4c1d9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -54,6 +54,7 @@ #include "virtypedparam.h" #include "virrandom.h" #include "virstring.h" +#include "cpu/cpu.h" #define VIR_FROM_THIS VIR_FROM_TEST @@ -5801,6 +5802,15 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; } +static int +testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, +const char *arch, +char ***models, +unsigned int flags) +{ +virCheckFlags(0, -1); +return cpuGetModels(arch, models); +} static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5872,6 +5882,7 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ +.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 6/7] virsh: add function to get the CPU models for an arch
Signed-off-by: Giuseppe Scrivano --- tools/virsh-host.c | 54 ++ tools/virsh.pod| 5 + 2 files changed, 59 insertions(+) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 880ae4b..5d47428 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -627,6 +627,54 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* + * "cpu-models" command + */ +static const vshCmdInfo info_cpu_models[] = { +{.name = "help", + .data = N_("CPU models") +}, +{.name = "desc", + .data = N_("Get the CPU models for an arch.") +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_cpu_models[] = { +{.name = "arch", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("architecture") +}, +{.name = NULL} +}; + +static bool +cmdCPUModelNames(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ +char **models; +size_t i; +int nmodels; +const char *arch = NULL; + +if (vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0) +return false; + +nmodels = virConnectGetCPUModelNames(ctl->conn, arch, &models, 0); +if (nmodels < 0) { +vshError(ctl, "%s", _("failed to get CPU model names")); +return false; +} + +for (i = 0; i < nmodels; i++) { +vshPrint(ctl, "%s\n", models[i]); +VIR_FREE(models[i]); +} +VIR_FREE(models); + +return true; +} + +/* * "version" command */ static const vshCmdInfo info_version[] = { @@ -851,6 +899,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_capabilities, .flags = 0 }, +{.name = "cpu-models", + .handler = cmdCPUModelNames, + .opts = opts_cpu_models, + .info = info_cpu_models, + .flags = 0 +}, {.name = "freecell", .handler = cmdFreecell, .opts = opts_freecell, diff --git a/tools/virsh.pod b/tools/virsh.pod index 0ae5178..72555e7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -163,6 +163,7 @@ group as an option. For example: Host and Hypervisor (help keyword 'host'): capabilities capabilities + cpu-models show the CPU models for an architecture connect(re)connect to hypervisor freecell NUMA free memory hostname print the hypervisor hostname @@ -358,6 +359,10 @@ current domain is in. =over 4 +=item B I + +Print the list of CPU models known for the specified architecture. + =item B The domain is currently running on a CPU -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/7] cpu: add function to get the models for an arch
Signed-off-by: Giuseppe Scrivano --- src/cpu/cpu.c| 56 src/cpu/cpu.h| 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 60 insertions(+) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 50c2de3..7011b3d 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -27,11 +27,13 @@ #include "viralloc.h" #include "virxml.h" #include "cpu.h" +#include "cpu_map.h" #include "cpu_x86.h" #include "cpu_powerpc.h" #include "cpu_s390.h" #include "cpu_arm.h" #include "cpu_generic.h" +#include "util/virstring.h" #define NR_DRIVERS ARRAY_CARDINALITY(drivers) @@ -456,3 +458,57 @@ cpuModelIsAllowed(const char *model, } return false; } + +struct cpuGetModelsData +{ +char **data; +size_t len; /* It includes the last element of DATA, which is NULL. */ +}; + +static int +cpuGetArchModelsCb(enum cpuMapElement element, + xmlXPathContextPtr ctxt, + void *cbdata) +{ +char *name; +struct cpuGetModelsData *data = cbdata; +if (element != CPU_MAP_ELEMENT_MODEL) +return 0; + +name = virXPathString("string(@name)", ctxt); +if (name == NULL) +return -1; + +return VIR_INSERT_ELEMENT(data->data, data->len - 1, data->len, name); +} + + +static int +cpuGetArchModels(const char *arch, struct cpuGetModelsData *data) +{ +return cpuMapLoad(arch, cpuGetArchModelsCb, data); +} + + +int +cpuGetModels(const char *arch, char ***models) +{ +struct cpuGetModelsData data; + +*models = data.data = NULL; +data.len = 1; + +if (VIR_ALLOC_N(data.data, data.len) < 0) +goto error; + +if (cpuGetArchModels(arch, &data) < 0) +goto error; + +*models = data.data; + +return data.len - 1; + +error: +virStringFreeList(data.data); +return -1; +} diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 69f6fae..ebbc7c1 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -175,4 +175,7 @@ cpuModelIsAllowed(const char *model, const char **models, unsigned int nmodels); +extern int +cpuGetModels(const char *arch, char ***models); + #endif /* __VIR_CPU_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..b456c2d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -719,6 +719,7 @@ cpuCompareXML; cpuDataFree; cpuDecode; cpuEncode; +cpuGetModels; cpuGuestData; cpuHasFeature; cpuMapOverride; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/7] add new API virConnectGetCPUModelNames
This series adds a new API "virConnectGetCPUModelNames" that allows to retrieve the list of CPU models known by the hypervisor for a specific architecture. This new function is mainly needed by virt-manager to not read directly the cpu_map.xml file (it could also be different when accessing a remote daemon). I have amended all the comments reported for v2. *v3 main changes - virConnectGetCPUModelNames returns the number of models instead of 0 on success. - Use VIR_INSERT_ELEMENT instead of VIR_EXPAND_N. - Fix a potential memory leak in the python bindings. - Move virsh changes to a separate commit. - Remove API documentation from libvirt.h. *v2 main changes - set a hard limit for the number of CPU models that is possible to fetch from a remote server. - Use VIR_EXPAND_N instead of VIR_REALLOC_N. - s|1.1.2|1.1.3| Giuseppe Scrivano (7): libvirt: add new public API virConnectGetCPUModelNames cpu: add function to get the models for an arch virConnectGetCPUModelNames: implement the remote protocol virConnectGetCPUModelNames: add the support for qemu virConnectGetCPUModelNames: add the support for the test protocol virsh: add function to get the CPU models for an arch python: add bindings for virConnectGetCPUModelNames daemon/remote.c | 43 +++ include/libvirt/libvirt.h.in| 4 +++ python/generator.py | 1 + python/libvirt-override-api.xml | 7 + python/libvirt-override.c | 52 + python/libvirt-override.py | 11 src/cpu/cpu.c | 56 src/cpu/cpu.h | 3 +++ src/driver.h| 7 + src/libvirt.c | 46 + src/libvirt_private.syms| 1 + src/libvirt_public.syms | 5 src/qemu/qemu_driver.c | 14 ++ src/remote/remote_driver.c | 57 + src/remote/remote_protocol.x| 20 ++- src/remote_protocol-structs | 11 src/test/test_driver.c | 11 tools/virsh-host.c | 54 ++ tools/virsh.pod | 5 19 files changed, 407 insertions(+), 1 deletion(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 7/7] python: add bindings for virConnectGetCPUModelNames
Signed-off-by: Giuseppe Scrivano --- python/generator.py | 2 +- python/libvirt-override-api.xml | 7 ++ python/libvirt-override.c | 52 + python/libvirt-override.py | 11 + 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/python/generator.py b/python/generator.py index 73107d7..12c14f1 100755 --- a/python/generator.py +++ b/python/generator.py @@ -250,6 +250,7 @@ lxc_functions_failed = [] qemu_functions_failed = [] functions_skipped = [ "virConnectListDomains", +"virConnectGetCPUModelNames", ] lxc_functions_skipped = [] qemu_functions_skipped = [] @@ -366,7 +367,6 @@ foreign_encoding_args = ( # Class methods which are written by hand in libvirt.c but the Python-level # code is still automatically generated (so they are not in skip_function()). skip_impl = ( -"virConnectGetCPUModelNames", 'virConnectGetVersion', 'virConnectGetLibVersion', 'virConnectListDomainsID', diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9a88215..1bceb05 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -476,6 +476,13 @@ + + Get the list of supported CPU models. + + + + + collect the list of snapshot names for the given domain diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d16b9a2..63c2566 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) diff --git a/python/libvirt-override.py b/python/libvirt-override.py index ccfec48..3471a43 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -207,3 +207,14 @@ def virEventAddTimeout(timeout, cb, opaque): ret = libvirtmod.virEventAddTimeout(timeout, cbData) if ret == -1: raise libvirtError ('virEventAddTimeout() failed') return ret + +def getCPUModelNames(conn, arch, flags=0): +""" +get the list of supported CPU models. +@conn: virConnect connection +@arch: Architecture +@flags: extra flags; not used yet, so callers should always pass 0. +""" +ret = libvirtmod.virConnectGetCPUModelNames(conn._o, arch, flags) +if ret == None: raise libvirtError ('virConnectGetCPUModelNames() failed', conn=self) +return ret -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/7] virConnectGetCPUModelNames: implement the remote protocol
Signed-off-by: Giuseppe Scrivano --- daemon/remote.c | 43 + src/remote/remote_driver.c | 57 src/remote/remote_protocol.x | 20 +++- src/remote_protocol-structs | 11 + 4 files changed, 130 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2aff7c1..d357016 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5037,6 +5037,49 @@ cleanup: static int +remoteDispatchConnectGetCPUModelNames(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_get_cpu_model_names_args *args, + remote_connect_get_cpu_model_names_ret *ret) +{ +int len, rv = -1; +char **models; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv->conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); +goto cleanup; +} + +len = virConnectGetCPUModelNames(priv->conn, args->arch, &models, + args->flags); +if (len < 0) +goto cleanup; + +if (ret->models.models_len > REMOTE_CONNECT_CPU_MODELS_MAX) { +virReportError(VIR_ERR_RPC, + _("Too many CPU models '%d' for limit '%d'"), + ret->models.models_len, REMOTE_CONNECT_CPU_MODELS_MAX); +virStringFreeList(models); +goto cleanup; +} + +ret->models.models_val = models; +ret->models.models_len = len; + +rv = 0; + +cleanup: +if (rv < 0) +virNetMessageSaveError(rerr); +return rv; +} + + +static int remoteDispatchDomainCreateXMLWithFiles(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 62e77a5..aa3762a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5541,6 +5541,62 @@ done: static int +remoteConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char ***models, + unsigned int flags) +{ +int rv = -1; +size_t i; +char **retmodels; +remote_connect_get_cpu_model_names_args args; +remote_connect_get_cpu_model_names_ret ret; +struct private_data *priv = conn->privateData; +remoteDriverLock(priv); + +memset(&args, 0, sizeof(args)); +memset(&ret, 0, sizeof(ret)); + +args.arch = (char *) arch; +args.flags = flags; + +if (call(conn, priv, 0, REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES, + (xdrproc_t) xdr_remote_connect_get_cpu_model_names_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_get_cpu_model_names_ret, + (char *) &ret) < 0) +goto error; + +/* Check the length of the returned list carefully. */ +if (ret.models.models_len > REMOTE_CONNECT_CPU_MODELS_MAX) { +virReportError(VIR_ERR_RPC, "%s", + _("remoteConnectGetCPUModelNames: " + "returned number of CPU models exceeds limit")); +goto error; +} + +if (VIR_ALLOC_N(retmodels, ret.models.models_len + 1) < 0) +goto error; + +for (i = 0; i < ret.models.models_len; i++) +retmodels[i] = ret.models.models_val[i]; + +/* Caller frees MODELS. */ +*models = retmodels; +rv = ret.models.models_len; + +done: +remoteDriverUnlock(priv); +return rv; + +error: +rv = -1; +virStringFreeList(ret.models.models_val); +goto done; +} + + +static int remoteDomainOpenGraphics(virDomainPtr dom, unsigned int idx, int fd, @@ -6933,6 +6989,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ +.connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.3 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index a1c23da..a1d90ad 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -232,6 +232,9 @@ const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64; /* Upper limit on numb
[libvirt] [PATCH v3 1/7] libvirt: add new public API virConnectGetCPUModelNames
The new function virConnectGetCPUModelNames allows to retrieve the list of CPU models known by the hypervisor for a specific architecture. Signed-off-by: Giuseppe Scrivano --- include/libvirt/libvirt.h.in | 4 python/generator.py | 1 + src/driver.h | 7 +++ src/libvirt.c| 46 src/libvirt_public.syms | 5 + 5 files changed, 63 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a47e33c..e27150d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4006,6 +4006,10 @@ int virConnectCompareCPU(virConnectPtr conn, const char *xmlDesc, unsigned int flags); +int virConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char ***models, + unsigned int flags); /** * virConnectBaselineCPUFlags diff --git a/python/generator.py b/python/generator.py index a91dde8..73107d7 100755 --- a/python/generator.py +++ b/python/generator.py @@ -366,6 +366,7 @@ foreign_encoding_args = ( # Class methods which are written by hand in libvirt.c but the Python-level # code is still automatically generated (so they are not in skip_function()). skip_impl = ( +"virConnectGetCPUModelNames", 'virConnectGetVersion', 'virConnectGetLibVersion', 'virConnectListDomainsID', diff --git a/src/driver.h b/src/driver.h index be64333..8cd164a 100644 --- a/src/driver.h +++ b/src/driver.h @@ -682,6 +682,12 @@ typedef char * unsigned int flags); typedef int +(*virDrvConnectGetCPUModelNames)(virConnectPtr conn, + const char *args, + char ***models, + unsigned int flags); + +typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1332,6 +1338,7 @@ struct _virDriver { virDrvDomainMigratePerform3Params domainMigratePerform3Params; virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; +virDrvConnectGetCPUModelNames connectGetCPUModelNames; }; diff --git a/src/libvirt.c b/src/libvirt.c index 665b30b..6d09cbd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18519,6 +18519,52 @@ error: /** + * virConnectGetCPUModelNames: + * + * @conn: virConnect connection + * @arch: Architecture + * @models: NULL terminated array of the CPU models supported for the specified + * architecture. Each element and the array itself must be freed by the caller + * with free. + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the list of supported CPU models for a specific architecture. + * + * Returns -1 on error, number of elements in @models on success. + */ +int +virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, + unsigned int flags) +{ +VIR_DEBUG("conn=%p, arch=%s, flags=%x", conn, arch, flags); +virResetLastError(); + +if (!VIR_IS_CONNECT(conn)) { +virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} +virCheckNonNullArgReturn(arch, -1); + +if (conn->driver->connectGetCPUModelNames) { +int ret; + +ret = conn->driver->connectGetCPUModelNames(conn, arch, models, flags); +if (ret < 0) +goto error; + +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(conn); +return -1; +} + + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..fe9b497 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -634,4 +634,9 @@ LIBVIRT_1.1.1 { virDomainSetMemoryStatsPeriod; } LIBVIRT_1.1.0; +LIBVIRT_1.1.3 { +global: +virConnectGetCPUModelNames; +} LIBVIRT_1.1.1; + # define new API here using predicted next version number -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/7] virConnectGetCPUModelNames: add the support for qemu
Signed-off-by: Giuseppe Scrivano --- src/qemu/qemu_driver.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bbf2d23..c139daa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15880,6 +15880,19 @@ qemuNodeSuspendForDuration(virConnectPtr conn, return nodeSuspendForDuration(target, duration, flags); } +static int +qemuConnectGetCPUModelNames(virConnectPtr conn, +const char *arch, +char ***models, +unsigned int flags) +{ +virCheckFlags(0, -1); +if (virConnectGetCPUModelNamesEnsureACL(conn) < 0) +return -1; + +return cpuGetModels(arch, models); +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -16067,6 +16080,7 @@ static virDriver qemuDriver = { .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ +.connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ }; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/5] cpu_models: add the support for the test protocol
"Daniel P. Berrange" writes: >> +static int >> +testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, >> +const char *arch ATTRIBUTE_UNUSED, >> +char ***models, >> +unsigned int flags) >> +{ >> +char **null_list; >> + >> +virCheckFlags(0, -1); >> +if (VIR_ALLOC_N(null_list, 1) < 0) >> +return -1; >> + >> +*models = null_list; >> +return 0; >> +} > > It'd be nice to have a non-NULL list for this IMHO. I'll use cpuGetModels here as well. It shouldn't be a problem to use the same data as the qemu driver. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/5] cpu_models: add the support for the test protocol
Signed-off-by: Giuseppe Scrivano --- src/test/test_driver.c | 16 1 file changed, 16 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c225618..03d9c96 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5801,6 +5801,21 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; } +static int +testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, +const char *arch ATTRIBUTE_UNUSED, +char ***models, +unsigned int flags) +{ +char **null_list; + +virCheckFlags(0, -1); +if (VIR_ALLOC_N(null_list, 1) < 0) +return -1; + +*models = null_list; +return 0; +} static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5872,6 +5887,7 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ +.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list