Re: [libvirt] [RFC] qemu: Redesigning guest CPU configuration
On Mon, Jun 22, 2015 at 17:09:22 +0100, Daniel P. Berrange wrote: On Mon, Jun 22, 2015 at 05:58:46PM +0200, Jiri Denemark wrote: However, knowing all the details about a guest CPU used by QEMU for a given CPU model on a specific machine type is not enough to enforce ABI stability. Without using -cpu Model,enforce (or an equivalent of checking filtered-features via QMP) QEMU may silently filter features it cannot provide on the current host. Even in case of TCG some features are not supported, e.g., -cpu SandyBridge will always fail to start in enforcing mode. Even doing something ugly and using the enforce mode only for new machine types is not going to work because after a QEMU upgrade new libvirt would be incompatible with older libvirt. I'm not sure I follow the scenario you're concerned with. Lets, say we have guest XML cpumodelSandyBridge/model/cpu and so we're using the new custom -cpu arg that QEMU supports. Are you saying that we won't be able to live migrate from the -cpu custom with new QEMU, to -cpu SandyBridge with old QEMU, even if the CPU seen by the guest is identical ? There are two more or less separate issues. The first one is switching from -cpu SandyBridge to -cpu custom. This should be safe and having the mapping between machine types and CPU model version should make both command lines compatible even during migration. The second one is adding enforce, i.e., -cpu SandyBridge,enforce or -cpu custom,enforce (libvirt will likely implement it in a different way but the effect will be the same) to make sure QEMU does not filter any CPU feature. Without enforce, QEMU may silently drop any feature requested by the SandyBridge CPU model. During migration, QEMU on both sides can filter different features in case the host CPUs, kernel versions or settings, QEMU versions, or BIOS settings differ. So we really need to start making sure QEMU does not filter anything. But we shouldn't do that for existing domains because they could fail to start or migrate because QEMU filtered some features and we didn't care so far. That said, I don't really see a way to do all this automatically without an explicit switch in a domain XML. Be it a new CPU mode or an attribute which would request enforcing ABI stability. I don't like the idea of adding more to the mode=custom|host-model|passthroug options, but perhaps we could signify this in a differnet way. I'm not a big fan of this either. For example, what we're really doing here is switching between use of libvirt and use of QEMU for CPU emulation. In similar cases, for other device types we use the driver element to identify the backend impl. So perhaps we could do a cpu ... driver name=libvirt|qemu/ /cpu To distinguish between use of libvirt and use of QEMU for the CPU model/feature handling ? Ideally if not specified, then we'd magically choose the best approach given the QEMU we have available This would cover the first issue (-cpu Model vs. -cpu custom), which I think can be done automatically and we shouldn't need any XML modifications for it. I'm more concerned about the second issue (enforcing ABI stability). We could automatically enable enforcing mode for new CPU models, but we need and explicit switch for existing models. I was thinking about an attribute for cpu or maybe even batter for model which would turn enforcing on. Something like cpu mode='custom' match='exact' model fallback='allow' check='strict|relaxed'.../model ... /cpu Any CPU model which is not currently known to libvirt could easily default to check='strict'. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] vz: implementation of attach/detach network devices
Changes from v1 Remove cleanup label and goto operator from prlsdkAttachNet() and prlsdkDetachNet() Replace it with return operator. Rename netMac variable to expectedMac in prlsdkGetNetIndex() Move prlsdkGetNetIndex() call after PrlVm_BeginEdit call in prlsdkDetachNet() function. Mikhail Feoktistov (1): vz: implementation of attach/detach network devices -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] qemu: Redesigning guest CPU configuration
On Mon, Jun 22, 2015 at 18:43:56 +0200, Jiri Denemark wrote: On Mon, Jun 22, 2015 at 17:09:22 +0100, Daniel P. Berrange wrote: On Mon, Jun 22, 2015 at 05:58:46PM +0200, Jiri Denemark wrote: However, knowing all the details about a guest CPU used by QEMU for a given CPU model on a specific machine type is not enough to enforce ABI stability. Without using -cpu Model,enforce (or an equivalent of checking filtered-features via QMP) QEMU may silently filter features it cannot provide on the current host. Even in case of TCG some features are not supported, e.g., -cpu SandyBridge will always fail to start in enforcing mode. Even doing something ugly and using the enforce mode only for new machine types is not going to work because after a QEMU upgrade new libvirt would be incompatible with older libvirt. I'm not sure I follow the scenario you're concerned with. Lets, say we have guest XML cpumodelSandyBridge/model/cpu and so we're using the new custom -cpu arg that QEMU supports. Are you saying that we won't be able to live migrate from the -cpu custom with new QEMU, to -cpu SandyBridge with old QEMU, even if the CPU seen by the guest is identical ? There are two more or less separate issues. The first one is switching from -cpu SandyBridge to -cpu custom. This should be safe and having the mapping between machine types and CPU model version should make both command lines compatible even during migration. The second one is adding enforce, i.e., -cpu SandyBridge,enforce or -cpu custom,enforce (libvirt will likely implement it in a different way but the effect will be the same) to make sure QEMU does not filter any CPU feature. Without enforce, QEMU may silently drop any feature requested by the SandyBridge CPU model. During migration, QEMU on both sides can filter different features in case the host CPUs, kernel versions or settings, QEMU versions, or BIOS settings differ. So we really need to start making sure QEMU does not filter anything. But we shouldn't do that for existing domains because they could fail to start or migrate because QEMU filtered some features and we didn't care so far. That said, I don't really see a way to do all this automatically without an explicit switch in a domain XML. Be it a new CPU mode or an attribute which would request enforcing ABI stability. I don't like the idea of adding more to the mode=custom|host-model|passthroug options, but perhaps we could signify this in a differnet way. I'm not a big fan of this either. For example, what we're really doing here is switching between use of libvirt and use of QEMU for CPU emulation. In similar cases, for other device types we use the driver element to identify the backend impl. So perhaps we could do a cpu ... driver name=libvirt|qemu/ /cpu To distinguish between use of libvirt and use of QEMU for the CPU model/feature handling ? Ideally if not specified, then we'd magically choose the best approach given the QEMU we have available This would cover the first issue (-cpu Model vs. -cpu custom), which I think can be done automatically and we shouldn't need any XML modifications for it. I'm more concerned about the second issue (enforcing ABI stability). We could automatically enable enforcing mode for new CPU models, but we need and explicit switch for existing models. I was thinking about an attribute for cpu or maybe even batter for model which would turn enforcing on. Something like cpu mode='custom' match='exact' model fallback='allow' check='strict|relaxed'.../model ... /cpu Any CPU model which is not currently known to libvirt could easily default to check='strict'. Actually, we could automatically update domain XML when reconnecting to it on libvirtd restart or when the domain is first started and remove all features filtered by QEMU from it and enforce it doesn't change on migration, save/restore, or next start. So we could turned check from relaxed to strict automatically even for old domains (but not on their first start). Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: simplify json parsing
On 06/22/2015 09:05 AM, Michal Privoznik wrote: On 22.06.2015 15:52, Eric Blake wrote: On 06/20/2015 01:42 PM, Eric Blake wrote: Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor_json.c | 164 --- 1 file changed, 61 insertions(+), 103 deletions(-) @@ -271,7 +269,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon, memset(msg, 0, sizeof(msg)); -exe = virJSONValueObjectGet(cmd, execute); +exe = virJSONValueObjectGetObject(cmd, execute); if (exe) { if (!(id = qemuMonitorNextCommandID(mon))) goto cleanup; This hunk is spurious (the 'execute' member is a string, not an object); I've removed it locally from my tree. Agreed. ACK to all the patches then. Thanks; pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] qemu: Redesigning guest CPU configuration
On Mon, Jun 22, 2015 at 05:58:46PM +0200, Jiri Denemark wrote: However, knowing all the details about a guest CPU used by QEMU for a given CPU model on a specific machine type is not enough to enforce ABI stability. Without using -cpu Model,enforce (or an equivalent of checking filtered-features via QMP) QEMU may silently filter features it cannot provide on the current host. Even in case of TCG some features are not supported, e.g., -cpu SandyBridge will always fail to start in enforcing mode. Even doing something ugly and using the enforce mode only for new machine types is not going to work because after a QEMU upgrade new libvirt would be incompatible with older libvirt. I'm not sure I follow the scenario you're concerned with. Lets, say we have guest XML cpumodelSandyBridge/model/cpu and so we're using the new custom -cpu arg that QEMU supports. Are you saying that we won't be able to live migrate from the -cpu custom with new QEMU, to -cpu SandyBridge with old QEMU, even if the CPU seen by the guest is identical ? That said, I don't really see a way to do all this automatically without an explicit switch in a domain XML. Be it a new CPU mode or an attribute which would request enforcing ABI stability. I don't like the idea of adding more to the mode=custom|host-model|passthroug options, but perhaps we could signify this in a differnet way. For example, what we're really doing here is switching between use of libvirt and use of QEMU for CPU emulation. In similar cases, for other device types we use the driver element to identify the backend impl. So perhaps we could do a cpu ... driver name=libvirt|qemu/ /cpu To distinguish between use of libvirt and use of QEMU for the CPU model/feature handling ? Ideally if not specified, then we'd magically choose the best approach given the QEMU we have available Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] parallels: implementation of attach/detach network devices
Changes from v1 Remove cleanup label and goto operator from prlsdkAttachNet() and prlsdkDetachNet() Replace it with return operator. Rename netMac variable to expectedMac in prlsdkGetNetIndex() Move prlsdkGetNetIndex() call after PrlVm_BeginEdit call in prlsdkDetachNet() function. Patch comment: In this patch we add VIR_DOMAIN_DEVICE_NET handlers implementation for domainAttachDevice and domainDetachDevice callbacks. As soon as we don't support this operation for hypervisor type domains, we implement this functionality for containers only. In detach procedure we find network device by MAC address. Because PrlVmDevNet_GetMacAddress() returns MAC as a UTF-8 encoded null-terminated string, we use memcmp() to compare it. Also we remove corresponding virtual network by prlsdkDelNetAdapter call. --- src/vz/vz_driver.c | 16 +++ src/vz/vz_sdk.c| 123 src/vz/vz_sdk.h|4 ++ 3 files changed, 143 insertions(+), 0 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index cef3c77..d9ddd4f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1117,6 +1117,14 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkAttachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network attach failed)); +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be attached), @@ -1186,6 +1194,14 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkDetachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network detach failed)); +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be detached), diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 090a3ad..6e6d8c9 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2914,6 +2914,129 @@ static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) PrlHandle_Free(vnet); } +int prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net) +{ +int ret = -1; +parallelsDomObjPtr privdom = dom-privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +if (!IS_CT(dom-def)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(network device cannot be attached)); +return ret; +} + +job = PrlVm_BeginEdit(privdom-sdkdom); +if (PRL_FAILED(waitJob(job))) +return ret; + +ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def)); +if (ret == 0) { +job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); +if (PRL_FAILED(waitJob(job))) { +return -1; +} +} + +return ret; +} + +static int +prlsdkGetNetIndex(PRL_HANDLE sdkdom, virDomainNetDefPtr net) +{ +int idx = -1; +PRL_RESULT pret; +PRL_UINT32 adaptersCount; +PRL_UINT32 i; +PRL_HANDLE adapter = PRL_INVALID_HANDLE; +PRL_UINT32 len; +char adapterMac[PRL_MAC_STRING_BUFNAME]; +char expectedMac[PRL_MAC_STRING_BUFNAME]; + +prlsdkFormatMac(net-mac, expectedMac); +pret = PrlVmCfg_GetNetAdaptersCount(sdkdom, adaptersCount); +prlsdkCheckRetGoto(pret, cleanup); + +for (i = 0; i adaptersCount; ++i) { + +pret = PrlVmCfg_GetNetAdapter(sdkdom, i, adapter); +prlsdkCheckRetGoto(pret, cleanup); + +len = sizeof(adapterMac); +memset(adapterMac, 0, sizeof(adapterMac)); +pret = PrlVmDevNet_GetMacAddress(adapter, adapterMac, len); +prlsdkCheckRetGoto(pret, cleanup); + +if (memcmp(adapterMac, expectedMac, PRL_MAC_STRING_BUFNAME)) { + +PrlHandle_Free(adapter); +adapter = PRL_INVALID_HANDLE; +continue; +} + +idx = i; +break; +} + + cleanup: +PrlHandle_Free(adapter); +return idx; +} + +static int prlsdkDelNetAdapter(PRL_HANDLE sdkdom, int idx) +{ +int ret = -1; +PRL_RESULT pret; +PRL_HANDLE sdknet = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetNetAdapter(sdkdom, idx, sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_Remove(sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +ret = 0; + + cleanup: +PrlHandle_Free(sdknet); +return ret; +} + +int prlsdkDetachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net) +{ +int ret = -1, idx = -1; +parallelsDomObjPtr privdom =
Re: [libvirt] [RFC] qemu: Redesigning guest CPU configuration
On Mon, Jun 22, 2015 at 18:43:56 +0200, Jiri Denemark wrote: On Mon, Jun 22, 2015 at 17:09:22 +0100, Daniel P. Berrange wrote: On Mon, Jun 22, 2015 at 05:58:46PM +0200, Jiri Denemark wrote: However, knowing all the details about a guest CPU used by QEMU for a given CPU model on a specific machine type is not enough to enforce ABI stability. Without using -cpu Model,enforce (or an equivalent of checking filtered-features via QMP) QEMU may silently filter features it cannot provide on the current host. Even in case of TCG some features are not supported, e.g., -cpu SandyBridge will always fail to start in enforcing mode. Even doing something ugly and using the enforce mode only for new machine types is not going to work because after a QEMU upgrade new libvirt would be incompatible with older libvirt. I'm not sure I follow the scenario you're concerned with. Lets, say we have guest XML cpumodelSandyBridge/model/cpu and so we're using the new custom -cpu arg that QEMU supports. Are you saying that we won't be able to live migrate from the -cpu custom with new QEMU, to -cpu SandyBridge with old QEMU, even if the CPU seen by the guest is identical ? There are two more or less separate issues. The first one is switching from -cpu SandyBridge to -cpu custom. This should be safe and having the mapping between machine types and CPU model version should make both command lines compatible even during migration. The second one is adding enforce, i.e., -cpu SandyBridge,enforce or -cpu custom,enforce (libvirt will likely implement it in a different way but the effect will be the same) to make sure QEMU does not filter any CPU feature. Without enforce, QEMU may silently drop any feature requested by the SandyBridge CPU model. During migration, QEMU on both sides can filter different features in case the host CPUs, kernel versions or settings, QEMU versions, or BIOS settings differ. So we really need to start making sure QEMU does not filter anything. But we shouldn't do that for existing domains because they could fail to start or migrate because QEMU filtered some features and we didn't care so far. That said, I don't really see a way to do all this automatically without an explicit switch in a domain XML. Be it a new CPU mode or an attribute which would request enforcing ABI stability. I don't like the idea of adding more to the mode=custom|host-model|passthroug options, but perhaps we could signify this in a differnet way. I'm not a big fan of this either. For example, what we're really doing here is switching between use of libvirt and use of QEMU for CPU emulation. In similar cases, for other device types we use the driver element to identify the backend impl. So perhaps we could do a cpu ... driver name=libvirt|qemu/ /cpu To distinguish between use of libvirt and use of QEMU for the CPU model/feature handling ? Ideally if not specified, then we'd magically choose the best approach given the QEMU we have available This would cover the first issue (-cpu Model vs. -cpu custom), which I think can be done automatically and we shouldn't need any XML modifications for it. I'm more concerned about the second issue (enforcing ABI stability). We could automatically enable enforcing mode for new CPU models, but we need and explicit switch for existing models. I was thinking about an attribute for cpu or maybe even batter for model which would turn enforcing on. Something like cpu mode='custom' match='exact' model fallback='allow' check='strict|relaxed'.../model ... /cpu Any CPU model which is not currently known to libvirt could easily default to check='strict'. Also any mode != custom or match == minimal would be strict by default since it's us specifying the exact CPU model. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] qemu: Redesigning guest CPU configuration
On Fri, Jun 19, 2015 at 15:43:02 +0100, Daniel P. Berrange wrote: On Fri, Jun 19, 2015 at 04:36:34PM +0200, Jiri Denemark wrote: On Thu, Jun 18, 2015 at 14:41:17 +0100, Daniel P. Berrange wrote: So either we need to define all existing CPU models in all their variants used for various machine types and have a mapping between (model without a version, machine type) to a specific version of the model (which may be quite hard) or we need to be able to distinguish between an existing domain and a new domain with no CPU model version. While host-model and host-passthrough CPU modes are easy because they are designed to change everytime a domain starts (which means we don't need to be able to distinguish between existing and new domains), custom CPU mode are tricky. Currently, the only at least a bit reasonable thing which came to my mind is to have a new CPU mode, but it still seems awkward so please share your ideas if you have any. Introducing a new CPU mode feels pretty unpleasant to me. Although it will certainly be tedious work, getting details of all the CPU variants for historical machine types should be doable I think. Yeah, I also prefer this variant but I was kind of hoping someone would come up with a bright idea which would safe me from all the work :-P Allow me to introduce you to perl and regexes :-P Haha, I'm afraid it's not as simple since the machine type specific tweaks to CPU models are done in the code, you can't just look at some data structures to get all the variants. Actually, it has changed and it's defined in macros now, but there is another way which does not need a lot of work :-) Using a slightly modified (to produce a bit more condensed output) x86-cpu-model-dump script from Eduardo and some shell scripting around it I fetched all 700+ combinations of CPU models and machine types and differences between subsequent variants of the same CPU model. So this part was not that bad. However, knowing all the details about a guest CPU used by QEMU for a given CPU model on a specific machine type is not enough to enforce ABI stability. Without using -cpu Model,enforce (or an equivalent of checking filtered-features via QMP) QEMU may silently filter features it cannot provide on the current host. Even in case of TCG some features are not supported, e.g., -cpu SandyBridge will always fail to start in enforcing mode. Even doing something ugly and using the enforce mode only for new machine types is not going to work because after a QEMU upgrade new libvirt would be incompatible with older libvirt. That said, I don't really see a way to do all this automatically without an explicit switch in a domain XML. Be it a new CPU mode or an attribute which would request enforcing ABI stability. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: Properly mark acl.html dependencies
The acl.html file includes aclperms.htmlinc which is generated. However, there's a typo in the Makefile which makes make fail to see the dependencies correctly. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index b7b49cb..b807a0b 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -163,7 +163,7 @@ EXTRA_DIST= \ sitemap.html.in aclperms.htmlinc \ todo.pl hvsupport.pl todo.cfg-example -acl.html:: $(srcdir)/aclperms.htmlinc +acl.html: $(srcdir)/aclperms.htmlinc $(srcdir)/aclperms.htmlinc: $(top_srcdir)/src/access/viraccessperm.h \ $(srcdir)/genaclperms.pl Makefile.am -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] vz: implementation of attach/detach network devices
In this patch we add VIR_DOMAIN_DEVICE_NET handlers implementation for domainAttachDevice and domainDetachDevice callbacks. As soon as we don't support this operation for hypervisor type domains, we implement this functionality for containers only. In detach procedure we find network device by MAC address. Because PrlVmDevNet_GetMacAddress() returns MAC as a UTF-8 encoded null-terminated string, we use memcmp() to compare it. Also we remove corresponding virtual network by prlsdkDelNetAdapter call. --- src/vz/vz_driver.c | 16 +++ src/vz/vz_sdk.c| 123 src/vz/vz_sdk.h|4 ++ 3 files changed, 143 insertions(+), 0 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index cef3c77..d9ddd4f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1117,6 +1117,14 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkAttachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network attach failed)); +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be attached), @@ -1186,6 +1194,14 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkDetachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network detach failed)); +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be detached), diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 090a3ad..6e6d8c9 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2914,6 +2914,129 @@ static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) PrlHandle_Free(vnet); } +int prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net) +{ +int ret = -1; +parallelsDomObjPtr privdom = dom-privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +if (!IS_CT(dom-def)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(network device cannot be attached)); +return ret; +} + +job = PrlVm_BeginEdit(privdom-sdkdom); +if (PRL_FAILED(waitJob(job))) +return ret; + +ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def)); +if (ret == 0) { +job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); +if (PRL_FAILED(waitJob(job))) { +return -1; +} +} + +return ret; +} + +static int +prlsdkGetNetIndex(PRL_HANDLE sdkdom, virDomainNetDefPtr net) +{ +int idx = -1; +PRL_RESULT pret; +PRL_UINT32 adaptersCount; +PRL_UINT32 i; +PRL_HANDLE adapter = PRL_INVALID_HANDLE; +PRL_UINT32 len; +char adapterMac[PRL_MAC_STRING_BUFNAME]; +char expectedMac[PRL_MAC_STRING_BUFNAME]; + +prlsdkFormatMac(net-mac, expectedMac); +pret = PrlVmCfg_GetNetAdaptersCount(sdkdom, adaptersCount); +prlsdkCheckRetGoto(pret, cleanup); + +for (i = 0; i adaptersCount; ++i) { + +pret = PrlVmCfg_GetNetAdapter(sdkdom, i, adapter); +prlsdkCheckRetGoto(pret, cleanup); + +len = sizeof(adapterMac); +memset(adapterMac, 0, sizeof(adapterMac)); +pret = PrlVmDevNet_GetMacAddress(adapter, adapterMac, len); +prlsdkCheckRetGoto(pret, cleanup); + +if (memcmp(adapterMac, expectedMac, PRL_MAC_STRING_BUFNAME)) { + +PrlHandle_Free(adapter); +adapter = PRL_INVALID_HANDLE; +continue; +} + +idx = i; +break; +} + + cleanup: +PrlHandle_Free(adapter); +return idx; +} + +static int prlsdkDelNetAdapter(PRL_HANDLE sdkdom, int idx) +{ +int ret = -1; +PRL_RESULT pret; +PRL_HANDLE sdknet = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetNetAdapter(sdkdom, idx, sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_Remove(sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +ret = 0; + + cleanup: +PrlHandle_Free(sdknet); +return ret; +} + +int prlsdkDetachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net) +{ +int ret = -1, idx = -1; +parallelsDomObjPtr privdom = dom-privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +if (!IS_CT(dom-def)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(network device cannot be detached)); +return ret; +} + +job = PrlVm_BeginEdit(privdom-sdkdom); +if
Re: [libvirt] [PATCH v2 1/2] Introduce QEMU_CAPS_ARM_VIRT_PCI
On 06/22/2015 04:21 AM, Peter Krempa wrote: On Sun, Jun 21, 2015 at 16:10:47 -0400, Cole Robinson wrote: On 06/11/2015 02:40 AM, Pavel Fedin wrote: This capability specifies that virt machine on ARM has PCI controller. Enabled when version is at least 2.3.0. Signed-off-by: Pavel Fedin p.fe...@samsung.com --- src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ca7a7c2..2eccc97 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -285,6 +285,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, dea-key-wrap, pci-serial, aarch64-off, + arm-virt-pci, ); @@ -1330,6 +1331,10 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); } +if (version = 2003000) { +virQEMUCapsSet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI); +} + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b5a7770..3c1a8b9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -229,6 +229,7 @@ typedef enum { QEMU_CAPS_DEA_KEY_WRAP = 187, /* -machine dea_key_wrap */ QEMU_CAPS_DEVICE_PCI_SERIAL = 188, /* -device pci-serial */ QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */ +QEMU_CAPS_ARM_VIRT_PCI = 190, /* ARM 'virt' machine has PCI bus */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; ACK and pushed, tweaked to avoid the conflict with .git (since additions to qemu_capabilities are always conflicting, it's better to get this in early) It breaks syntax-check since the 'if' has a single line body with braces around it. Also pushing a capabiltity without the code that will actually use it is not exactly a good idea since it might never be used if the next patch gets abandoned and since they are considered public we might be stuck with it forever. hmm sorry, I suck with syntax-check... but WRT to this specific capability, I don't know how we won't end up using it, so in this case I think it's pretty safe. But I won't push a check again without it's accompanying usage. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: simplify json parsing
On 06/22/2015 11:01 AM, Eric Blake wrote: On 06/22/2015 09:05 AM, Michal Privoznik wrote: On 22.06.2015 15:52, Eric Blake wrote: On 06/20/2015 01:42 PM, Eric Blake wrote: Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with. Agreed. ACK to all the patches then. Thanks; pushed. And now seeing a test failure on 'jsontest' on a RHEL 6 box :( I'll investigate, since I just caused a build failure. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] lxc: setsid() usage
On Mon, Jun 22, 2015 at 04:40:37PM +0200, Richard Weinberger wrote: Hi! Why is libvirt-lxc issuing a setsid() in lxcContainerSetupFDs()? To me it seems like a hack to have a controlling TTY if PID 1 is /bin/bash. I honestly can't remember the reason. It might have been to ensure we have separation from the libvirt_lxc session. If one runs a sysv init style distro (like Debian) in libvirt-lxc the setsid() has a major downside, when getty spawns a login shell on /dev/tty1 it cannot become the controlling tty. Hence, if one presses ctrl-c in such a session, the container will reboot. Is that problem due to the fact we call setsid(), or due to use calling ioctl(TIOCSCTTY) ? Interestingly it does not happen when a systemd distro is used. Maybe because systemd completely closes and reopens the TTY? I have a feeling it does close reopen the tty, but i dunno if that has an impact on the ability to set the controlling tty ? Also note systemd uses the device via /dev/console, not /dev/tty1 and with 'container_ttys' we've told it not to use /dev/tty1 for gettys. So maybe it deals with /dev/console in a different way than it would if it were /dev/tty1 Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: simplify json parsing
On 22.06.2015 15:52, Eric Blake wrote: On 06/20/2015 01:42 PM, Eric Blake wrote: Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor_json.c | 164 --- 1 file changed, 61 insertions(+), 103 deletions(-) @@ -271,7 +269,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon, memset(msg, 0, sizeof(msg)); -exe = virJSONValueObjectGet(cmd, execute); +exe = virJSONValueObjectGetObject(cmd, execute); if (exe) { if (!(id = qemuMonitorNextCommandID(mon))) goto cleanup; This hunk is spurious (the 'execute' member is a string, not an object); I've removed it locally from my tree. Agreed. ACK to all the patches then. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
The first 4 patches are bugfixes/reorganizations that have no controversy. The sets of 5-7, 8-10, and 11-13 each implement a new model of PCI controller: 5-7 - controller type='pci' model='pcie-root-port'/ This is based on qemu's ioh3420. 8-10 - controller type='pci' model='pcie-switch-upstream-port'/ Based on qemu's x3130-upstream 11-13 - controller type='pci' model='pcie-switch-downstream-port'/ (xio3130-downstream) The first patch of each set adds a capability bit for qemu (again non-controversial), the 2nd adds the new pci controller model, and the 3rd implements that model in qemu (by checking for the capability and adding a commandline arg or failing). The controversial/RFC bit is this - talking to Alex Williamson (after I'd rwritten these patches, which is why I'm presenting them in a form that I *don't* want to push) about the possibility of qemu adding generic root-port, switch-upstream-port, and switch-downstream-port controllers, and possibly also a generic dmi-to-pci-bridge (i.e. controllers not tied to any particular hardware implementation as with those currently available), I'm realizing that, while it was a correct decision to make all of the existing pci controllers type='pci' (since they share an address space), using the model attribute to set the kind of controller was probably a mistake. The problem - if: controller type='pci' model='dmi-to-pci-bridge'/ currently means to add an i82801b11-bridge controller to the domain, once qemu implements a generic dmi-to-pci-bridge, how will *that* be denoted, and how will we avoid replacing the existing i81801b11-bridge in a particular domain with the generic version when a guest is restarted following a qemu/libvirt upgrade? In hindsight, it probably would have been better to do something like this with the four existing pci controllers: controller type='pci' subType='dmi-to-pci-bridge' model='i82801b11-bridge'/ controller type='pci' subType='pci-bridge' model='pci-bridge'/ (or maybe blank?) controller type='pci' subType='pci-root'/ (again maybe model is blank) controller type='pci' subType='pcie-root'/(and again) (instead, what is shown above as subType is currently placed in the model attribute). Then we could add the 3 new types like this: controller type='pci' subType='pcie-root-port' model='ioh3420'/ controller type='pci' subType='pcie-switch-upstream-port' model='x3130-upstream/ controller type='pci' subType='pcie-switch-downstream-port' model='xio3130-downstream/ and we would easily be able to add support for new generic controllers that behaved identically, by just adding a new model. But we haven't done that, and anything we do in the future must be backwards compatible with what's already there (right?). I'm trying to think of how to satisfy backward compatibility while making things work better in the future. Some ideas, in no particular order: === Idea 1: multiplex the meaning of the model attribute, so we currently have: controller type='pci' model='dmi-to-pci-bridge'/ which means add an i82801b11-bridge device and when we add a generic version of this type of controller, we would do it with something like: controller type='pci' model='generic-dmi-to-pci-bridge'/ and for another vendor's mythical controller: controller type='pci' model='xyz-dmi-to-pci-bridge'/ Cons: This will make for ugliness in switch statements where a new case will have to be added whenever a different controller with similar behavior/usage is supported. And it's generally not a good idea to have a single attribute be used for two different functions. === Idea 2: implement new controllers as suggested in 20/20 hindsight above. For controllers in existing domains (dmi-to-pci-bridge, pic-bridge, pci-root, and pcie-root) imply it into the controller definition of an existing domain when only model has been given (but don't write it out that way, to preserve the ability to downgrade). So this: [1] controller type='pci' model='dmi-to-pci-bridge'/ would internally mean this: [2] controller type='pci' subType='dmi-to-pci-bridge' model='i82801b11-bridge'/ (but would remain as [1] when config is rewritten/migrated) while this: [3] controller type='pci' subType='dmi-to-pci-bridge' model='anything whatsoever/ would mean exactly what it says. Cons: Keeping this straight would mean having some sort of oldStyleCompat flag in the controller object, to be sure that [1] wasn't sent in migration status as [2] (since the destination might not recognize it). It would also mean keeping the code in the parser and formatter to deal with this flag. Forever. === Idea 3: interpret controllers with missing subType as above, but actually write it out to the config/migration/etc in the new modified format. Cons: This would prevent downgrading libvirt
[libvirt] [PATCH 05/13] qemu: add capabilities bit for device ioh3420
This is a PCIE root port. It connects only to a port of the integrated pcie.0 bus of a Q35 machine (can't be hotplugged), and provides a single PCIe port that can have PCI or PCIe devices hotplugged into it. This device will be used to implement the pcie-root-port model of pci controller. --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_capabilities.h | 3 ++- tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 6 -- 10 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 27a632a..a9a19e1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1,7 +1,7 @@ /* * qemu_capabilities.c: QEMU capabilities generation * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, aarch64-off, vhost-user-multiqueue, /* 190 */ + ioh3420, ); @@ -1566,6 +1567,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { ivshmem, QEMU_CAPS_DEVICE_IVSHMEM }, { pc-dimm, QEMU_CAPS_DEVICE_PC_DIMM }, { pci-serial, QEMU_CAPS_DEVICE_PCI_SERIAL }, +{ ioh3420, QEMU_CAPS_DEVICE_IOH3420 }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 30aa504..d38505e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -1,7 +1,7 @@ /* * qemu_capabilities.h: QEMU capabilities generation * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -230,6 +230,7 @@ typedef enum { QEMU_CAPS_DEVICE_PCI_SERIAL = 188, /* -device pci-serial */ QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */ QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */ +QEMU_CAPS_DEVICE_IOH3420 = 191, /* -device ioh3420 */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index 30239df..a1fafa6 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -120,4 +120,5 @@ flag name='vmware-svga.vgamem_mb'/ flag name='qxl.vgamem_mb'/ flag name='qxl-vga.vgamem_mb'/ +flag name='ioh3420'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index ea3d850..824ef02 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -135,4 +135,5 @@ flag name='qxl.vgamem_mb'/ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ +flag name='ioh3420'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index 2c19ddc..7ef5199 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -136,4 +136,5 @@ flag name='qxl.vgamem_mb'/ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ +flag name='ioh3420'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index aadccd5..8c3d48e 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -145,4 +145,5 @@ flag name='qxl.vgamem_mb'/ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ +flag name='ioh3420'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 3e81cbf..72f7625 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -151,4 +151,5 @@ flag name='qxl.vgamem_mb'/ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ +flag name='ioh3420'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index 84c357f..d81c80c 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -151,4 +151,5 @@ flag name='qxl.vgamem_mb'/
[libvirt] [PATCH 03/13] qemu: ignore assumptions about hotplug requirement when address is from config
Certain PCI buses don't support hotplug, and when automatically assigning PCI addresses for devices, libvirt is very concervative in its assumptions about whether or not a device will need to be hotplugged/unplugged in the future. But if the user manually assigns an address, they likely are aware of any hotplug requirements of the device (or at least they should be). In short, after this patch, automatically PCI address assignment will assume that the device must be pluggedin ot a hot-pluggable slot, but manually assignment can place the device in any bus that is compatible, regardless of whether or not it supports hotplug. If the user makes a mistake and plugs the device into a bus that doesn't support hotplug, then later tries to do a hot-unplug, qemu will give an appropriate error. (in the future we may want to add a hotpluggable attribute to all devices, with default being yes for autoassign, and no for manual assign). --- src/conf/domain_addr.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 93c6043..2be98c5 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -50,6 +50,12 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, */ if (busFlags VIR_PCI_CONNECT_TYPES_ENDPOINT) busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT; +/* Also allow manual specification of bus to override + * libvirt's assumptions about whether or not hotplug + * capability will be required. + */ +if (devFlags VIR_PCI_CONNECT_HOTPLUGGABLE) +busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE; } /* If this bus doesn't allow the type of connection (PCI -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/13] docs: document when pcie-root/dmi-to-pci-bridge support was added
Also move the mention of version numbers for the various PCI controller models up to the end of the sentence where they are first given, to avoid confusion. --- docs/formatdomain.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1dca2ac..fb3e2fb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2998,6 +2998,9 @@ PCI controllers have an optional codemodel/code attribute with possible values codepci-root/code, codepcie-root/code, codepci-bridge/code, or codedmi-to-pci-bridge/code. + (pci-root and pci-bridge span class=sincesince 1.0.5/span, + pcie-root and dmi-to-pci-bridge span class=sincesince + 1.1.2/span) The root controllers (codepci-root/code and codepcie-root/code) have an optional codepcihole64/code element specifying how big (in kilobytes, or in the unit specified by codepcihole64/code's @@ -3017,7 +3020,6 @@ only refer to PCI buses provided by already specified PCI controllers. Leaving gaps in the PCI controller indexes might lead to an invalid configuration. - (pci-root and pci-bridge span class=sincesince 1.0.5/span) /p pre ... -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] json: reject trailing garbage
Yajl 2 has a nice feature that it can be configured whether to allow multiple JSON objects parsed from a single stream, defaulting to off. And yajl 1.0.12 at least provided a way to tell if all input bytes were parsed, or if trailing bytes remained after a valid JSON object was parsed. But we target RHEL 6 yajl 1.0.7, which has neither of these. So fake it by always parsing '[...]' instead, so that trailing garbage either trips up the array parse, or is easily detected when unwrapping the result. * src/util/virjson.c (virJSONValueFromString): With older json, wrap text to avoid trailing garbage. * tests/jsontest.c (mymain): Add tests for this. Signed-off-by: Eric Blake ebl...@redhat.com --- src/util/virjson.c | 29 - tests/jsontest.c | 2 ++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 8d12fad..a33005a 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1597,6 +1597,7 @@ virJSONValueFromString(const char *jsonstring) size_t len = strlen(jsonstring); # ifndef WITH_YAJL2 yajl_parser_config cfg = { 0, 1 }; +virJSONValuePtr tmp; # endif VIR_DEBUG(string=%s, jsonstring); @@ -1616,7 +1617,21 @@ virJSONValueFromString(const char *jsonstring) goto cleanup; } +/* Yajl 2 is nice enough to default to rejecting trailing garbage. + * Yajl 1.0.12 has yajl_get_bytes_consumed to make that detection + * simpler. But we're stuck with yajl 1.0.7 on RHEL 6, which + * happily quits parsing at the end of a valid JSON construct, + * with no visibility into how much more input remains. Wrapping + * things in an array forces yajl to confess the truth. */ +# ifdef WITH_YAJL2 rc = yajl_parse(hand, (const unsigned char *)jsonstring, len); +# else +rc = yajl_parse(hand, (const unsigned char *)[, 1); +if (VIR_YAJL_STATUS_OK(rc)) +rc = yajl_parse(hand, (const unsigned char *)jsonstring, len); +if (VIR_YAJL_STATUS_OK(rc)) +rc = yajl_parse(hand, (const unsigned char *)], 1); +# endif if (!VIR_YAJL_STATUS_OK(rc) || yajl_complete_parse(hand) != yajl_status_ok) { unsigned char *errstr = yajl_get_error(hand, 1, @@ -1638,6 +1653,18 @@ virJSONValueFromString(const char *jsonstring) virJSONValueFree(parser.head); } else { ret = parser.head; +# ifndef WITH_YAJL2 +/* Undo the array wrapping above */ +tmp = ret; +ret = NULL; +if (virJSONValueArraySize(tmp) 1) +virReportError(VIR_ERR_INTERNAL_ERROR, + _(cannot parse json %s: too many items present), + jsonstring); +else +ret = virJSONValueArraySteal(tmp, 0); +virJSONValueFree(tmp); +# endif } cleanup: @@ -1650,7 +1677,7 @@ virJSONValueFromString(const char *jsonstring) VIR_FREE(parser.state); } -VIR_DEBUG(result=%p, parser.head); +VIR_DEBUG(result=%p, ret); return ret; } diff --git a/tests/jsontest.c b/tests/jsontest.c index f6c2d84..a363dc0 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -419,6 +419,8 @@ mymain(void) DO_TEST_PARSE_FAIL(overdone keyword, [ truest ]); DO_TEST_PARSE_FAIL(unknown keyword, huh); DO_TEST_PARSE_FAIL(comments, [ /* nope */\n1 // not this either\n]); +DO_TEST_PARSE_FAIL(trailing garbage, [] []); +DO_TEST_PARSE_FAIL(list without array, 1, 1); DO_TEST_PARSE_FAIL(object with numeric keys, { 1:1, 2:1, 3:2 }); DO_TEST_PARSE_FAIL(unterminated object, { \1\:1, \2\:1, \3\:2); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5 v2] Corrections to SCSI logical unit handling
On 06/16/2015 11:29 PM, Eric Farman wrote: While working with the hostdev tag and SCSI LUNs, a problem was discovered with the XML schema (see commit message in patch 4). This spawned some further corrections to the handling of the logical unit field throughout libvirt. This series was split from a single patch, from this feedback: http://www.redhat.com/archives/libvir-list/2015-June/msg00489.html Eric Farman (5): Print SCSI logical unit as a positive integer Print SCSI logical unit as unsigned integer Convert SCSI logical unit from int to long long docs: Fix XML schema handling of LUN address in hostdev tag docs: Correct typos in scsi hostdev and address elements docs/formatdomain.html.in | 10 +++--- docs/schemas/domaincommon.rng | 14 -- src/conf/domain_audit.c | 2 +- src/conf/domain_conf.c| 4 ++-- src/conf/domain_conf.h| 2 +- src/qemu/qemu_command.h | 2 +- src/qemu/qemu_hotplug.c | 4 ++-- src/util/virhostdev.c | 6 +++--- src/util/virscsi.c| 16 src/util/virscsi.h| 8 tests/testutilsqemu.c | 2 +- tools/virsh-domain.c | 6 +++--- 12 files changed, 45 insertions(+), 31 deletions(-) FYI: Just pushed upstream. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] json: cope with older yajl semantics
Commit ceb496e5 fails on RHEL 6, with yajl 1.0.7, because that version of yajl returns yajl_status_insufficient_data when the parser is waiting for the rest of a token (this enum value was dropped in yajl 2, so we have to wrap it). It also exposes a problem where older yajl silently ignores trailing garbage after a successful parse, so this patch works around that by changing the testsuite. Another more invasive patch can add tighter semantics to json parsing, but this is sufficient for a minimal clean backport. While touching this, fix up our error message cleanup. Yajl documents that error messages produced by yajl_get_error() MUST be cleaned with yajl_free_error(); this is certainly true if we were to pass non-NULL allocator callbacks during yajl_alloc(), but probably harmless in our usage of passing NULL. But better safe than sorry. * src/util/virjson.c (virJSONValueFromString): Allow different error code. Use canonical cleanup of error message. (VIR_YAJL_STATUS_OK): New helper macro. * tests/jsontest.c (mymain): Wrap text to avoid difference in trailing garbage handling Signed-off-by: Eric Blake ebl...@redhat.com --- src/util/virjson.c | 12 tests/jsontest.c | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 29e2c39..4257b30 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -36,9 +36,12 @@ # ifdef WITH_YAJL2 # define yajl_size_t size_t +# define VIR_YAJL_STATUS_OK(status) ((status) == yajl_status_ok) # else # define yajl_size_t unsigned int # define yajl_complete_parse yajl_parse_complete +# define VIR_YAJL_STATUS_OK(status) \ +((status) == yajl_status_ok || (status) == yajl_status_insufficient_data) # endif #endif @@ -1590,6 +1593,8 @@ virJSONValueFromString(const char *jsonstring) yajl_handle hand; virJSONParser parser = { NULL, NULL, 0 }; virJSONValuePtr ret = NULL; +int rc; +size_t len = strlen(jsonstring); # ifndef WITH_YAJL2 yajl_parser_config cfg = { 1, 1 }; # endif @@ -1611,9 +1616,8 @@ virJSONValueFromString(const char *jsonstring) goto cleanup; } -if (yajl_parse(hand, - (const unsigned char *)jsonstring, - strlen(jsonstring)) != yajl_status_ok || +rc = yajl_parse(hand, (const unsigned char *)jsonstring, len); +if (!VIR_YAJL_STATUS_OK(rc) || yajl_complete_parse(hand) != yajl_status_ok) { unsigned char *errstr = yajl_get_error(hand, 1, (const unsigned char*)jsonstring, @@ -1622,7 +1626,7 @@ virJSONValueFromString(const char *jsonstring) virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse json %s: %s), jsonstring, (const char*) errstr); -VIR_FREE(errstr); +yajl_free_error(hand, errstr); virJSONValueFree(parser.head); goto cleanup; } diff --git a/tests/jsontest.c b/tests/jsontest.c index 34a07ee..8ac0970 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -416,7 +416,7 @@ mymain(void) DO_TEST_PARSE(boolean, true); DO_TEST_PARSE(null, null); DO_TEST_PARSE_FAIL(incomplete keyword, tr); -DO_TEST_PARSE_FAIL(overdone keyword, truest); +DO_TEST_PARSE_FAIL(overdone keyword, [ truest ]); DO_TEST_PARSE_FAIL(unknown keyword, huh); DO_TEST_PARSE_FAIL(object with numeric keys, { 1:1, 2:1, 3:2 }); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/3] json: even stricter trailing garbage detection
Since older yajl ignores trailing garbage, a client can cause problems by intentionally ending the wrapper array early. Since we already track nesting, it's not too much harder to reject invalid nesting pops. * src/util/virjson. (_virJSONParser): Add field. (virJSONValueFromString): Set witness. (virJSONParserHandleEndArray): Use it to catch abuse. * tests/jsontest.c (mymain): Test it. Signed-off-by: Eric Blake ebl...@redhat.com --- Could be squashed with 3/3, if desired. src/util/virjson.c | 7 +-- tests/jsontest.c | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index a33005a..3c6ed34 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -64,6 +64,7 @@ struct _virJSONParser { virJSONValuePtr head; virJSONParserStatePtr state; size_t nstate; +int wrap; }; @@ -1556,7 +1557,7 @@ virJSONParserHandleEndArray(void *ctx) VIR_DEBUG(parser=%p, parser); -if (!parser-nstate) +if (!(parser-nstate - parser-wrap)) return 0; state = (parser-state[parser-nstate-1]); @@ -1591,7 +1592,7 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring) { yajl_handle hand; -virJSONParser parser = { NULL, NULL, 0 }; +virJSONParser parser = { NULL, NULL, 0, 0 }; virJSONValuePtr ret = NULL; int rc; size_t len = strlen(jsonstring); @@ -1627,8 +1628,10 @@ virJSONValueFromString(const char *jsonstring) rc = yajl_parse(hand, (const unsigned char *)jsonstring, len); # else rc = yajl_parse(hand, (const unsigned char *)[, 1); +parser.wrap = 1; if (VIR_YAJL_STATUS_OK(rc)) rc = yajl_parse(hand, (const unsigned char *)jsonstring, len); +parser.wrap = 0; if (VIR_YAJL_STATUS_OK(rc)) rc = yajl_parse(hand, (const unsigned char *)], 1); # endif diff --git a/tests/jsontest.c b/tests/jsontest.c index a363dc0..97b9c0a 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -421,6 +421,7 @@ mymain(void) DO_TEST_PARSE_FAIL(comments, [ /* nope */\n1 // not this either\n]); DO_TEST_PARSE_FAIL(trailing garbage, [] []); DO_TEST_PARSE_FAIL(list without array, 1, 1); +DO_TEST_PARSE_FAIL(parser abuse, 1] [2); DO_TEST_PARSE_FAIL(object with numeric keys, { 1:1, 2:1, 3:2 }); DO_TEST_PARSE_FAIL(unterminated object, { \1\:1, \2\:1, \3\:2); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] conf: Enforce SCSI hostdev address type
If a SCSI subsystem hostdev element is provided with an address, then enforce that the address type is 'drive'. If not provided, a 'drive' element was created by virDomainHostdevAssignAddress which uses VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 4 ++-- src/conf/domain_conf.c| 6 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 95d8c45..0475527 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3343,8 +3343,8 @@ (starting with 0x) or octal (starting with 0) form. For PCI devices the element carries 4 attributes allowing to designate the device as can be found with the codelspci/code or - with codevirsh - nodedev-list/code. a href=#elementsAddressSee above/a for + with codevirsh nodedev-list/code. For SCSI devices a 'drive' + address type is used. a href=#elementsAddressSee above/a for more details on the address element./dd dtcodedriver/code/dt dd diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e592adf..ce5093d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11752,6 +11752,12 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, virReportError(VIR_ERR_XML_ERROR, %s, _(SCSI host devices must have address specified)); goto error; +} else if (def-info-type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(SCSI host device must use 'drive' + address type)); +goto error; } if (virXPathBoolean(boolean(./readonly), ctxt)) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] conf: Check for hostdev conflicts when assign default disk address
https://bugzilla.redhat.com/show_bug.cgi?id=1210587 (completed) When generating the default drive address for a SCSI disk device, check the generated address to ensure it doesn't conflict with a SCSI hostdev address. The disk address generation algorithm uses the target dev name in order to determine which controller and unit in order to place the device. Since a SCSI hostdev device doesn't require a target device name, its placement on the guest SCSI address could conflict. For instance, if a SCSI hostdev exists at controller=0 unit=0 and an attempt to hotplug 'sda' into the guest made, there would be a conflict if the hostdev is already using /dev/sda. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 40 +--- src/conf/domain_conf.h | 3 ++- src/qemu/qemu_command.c | 4 ++-- src/vmx/vmx.c | 22 -- src/vmx/vmx.h | 3 ++- 5 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6259d4a..0999e86 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5672,7 +5672,8 @@ virDomainDiskFindByBusAndDst(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, - virDomainDiskDefPtr def) + virDomainDiskDefPtr def, + const virDomainDef *vmdef) { int idx = virDiskNameToIndex(def-dst); if (idx 0) { @@ -5683,7 +5684,10 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, } switch (def-bus) { -case VIR_DOMAIN_DISK_BUS_SCSI: +case VIR_DOMAIN_DISK_BUS_SCSI: { +unsigned int controller; +unsigned int unit; + def-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; if (xmlopt-config.hasWideSCSIBus) { @@ -5692,22 +5696,36 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, * Unit 7 is the SCSI controller itself. Therefore unit 7 * cannot be assigned to disks and is skipped. */ -def-info.addr.drive.controller = idx / 15; -def-info.addr.drive.bus = 0; -def-info.addr.drive.unit = idx % 15; +controller = idx / 15; +unit = idx % 15; /* Skip the SCSI controller at unit 7 */ -if (def-info.addr.drive.unit = 7) -++def-info.addr.drive.unit; +if (unit = 7) +++unit; } else { /* For a narrow SCSI bus we define the default mapping to be * 7 units per bus, 1 bus per controller, many controllers */ -def-info.addr.drive.controller = idx / 7; -def-info.addr.drive.bus = 0; -def-info.addr.drive.unit = idx % 7; +controller = idx / 7; +unit = idx % 7; +} + +if (virDomainDriveAddressIsUsedByHostdev(vmdef, + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI, + controller, 0, 0, unit)) { +virReportError(VIR_ERR_XML_ERROR, + _(address generated using disk target name '%s' + may conflict with SCSI host device address + controller='%u' bus='%u' target='%u' unit='%u), + def-dst, controller, 0, 0, unit); +return -1; } +def-info.addr.drive.controller = controller; +def-info.addr.drive.bus = 0; +def-info.addr.drive.target = 0; +def-info.addr.drive.unit = unit; break; +} case VIR_DOMAIN_DISK_BUS_IDE: /* For IDE we define the default mapping to be 2 units @@ -7261,7 +7279,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, if (!(flags VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { if (def-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE - virDomainDiskDefAssignAddress(xmlopt, def) 0) + virDomainDiskDefAssignAddress(xmlopt, def, vmdef) 0) goto error; if (virDomainDiskBackingStoreParse(ctxt, def-src) 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c96a6e4..5b1c838 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2717,7 +2717,8 @@ int virDomainDiskInsert(virDomainDefPtr def, void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, - virDomainDiskDefPtr def); + virDomainDiskDefPtr def, + const virDomainDef *vmdef); virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5444638..11e8e16 100644 ---
[libvirt] [PATCH 3/5] conf: Add SCSI hostdev check for disk drive address already in use
https://bugzilla.redhat.com/show_bug.cgi?id=1210587 (partial) If a SCSI subsystem hostdev element address is provided, we need to make sure the address provided doesn't conflict with an existing or libvirt generated address for a SCSI disk element. This will fix the issue where the domain XML provided an address for the hostdev, but not the disk element where the address provided ends up being the same address used for the disk. A disk address is generated using it's assigned target 'dev' name prior to the check/validation of the hostdev address value. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 31 ++- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3205c43..e02cd49 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11754,18 +11754,39 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: -if (def-info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE -virDomainHostdevAssignAddress(xmlopt, vmdef, def) 0) { +if (def-info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { +if (virDomainHostdevAssignAddress(xmlopt, vmdef, def) 0) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(SCSI host devices must have address specified)); -goto error; +virReportError(VIR_ERR_XML_ERROR, %s, + _(Failed to assign SCSI host + device address)); +goto error; +} } else if (def-info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { virReportError(VIR_ERR_XML_ERROR, %s, _(SCSI host device must use 'drive' address type)); goto error; +} else { +/* Ensure provided address doesn't conflict with existing + * scsi disk drive address + */ +virDomainDeviceDriveAddressPtr addr = def-info-addr.drive; +if (virDomainDriveAddressIsUsedByDisk(vmdef, + VIR_DOMAIN_DISK_BUS_SCSI, + addr-controller, + addr-bus, + addr-target, + addr-unit)) { +virReportError(VIR_ERR_XML_ERROR, + _(SCSI host address controller='%u' + bus='%u' target='%u' unit='%u' in + use by a SCSI disk), + addr-controller, addr-bus, + addr-target, addr-unit); +goto error; +} } if (virXPathBoolean(boolean(./readonly), ctxt)) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] conf: Add 'bus' and 'target' to SCSI address conflict checks
Modify virDomainDriveAddressIsUsedBy{Disk|Hostdev} and virDomainSCSIDriveAddressIsUsed to take 'bus' and 'target' parameters. Will be used by future patches for more complete address conflict checks Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce5093d..3205c43 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5183,6 +5183,8 @@ static bool virDomainDriveAddressIsUsedByDisk(const virDomainDef *def, virDomainDiskBus type, unsigned int controller, + unsigned int bus, + unsigned int target, unsigned int unit) { virDomainDiskDefPtr disk; @@ -5197,8 +5199,8 @@ virDomainDriveAddressIsUsedByDisk(const virDomainDef *def, if (disk-info.addr.drive.controller == controller disk-info.addr.drive.unit == unit -disk-info.addr.drive.bus == 0 -disk-info.addr.drive.target == 0) +disk-info.addr.drive.bus == bus +disk-info.addr.drive.target == target) return true; } @@ -5212,6 +5214,8 @@ static bool virDomainDriveAddressIsUsedByHostdev(const virDomainDef *def, virDomainHostdevSubsysType type, unsigned int controller, + unsigned int bus, + unsigned int target, unsigned int unit) { virDomainHostdevDefPtr hostdev; @@ -5225,8 +5229,8 @@ virDomainDriveAddressIsUsedByHostdev(const virDomainDef *def, if (hostdev-info-addr.drive.controller == controller hostdev-info-addr.drive.unit == unit -hostdev-info-addr.drive.bus == 0 -hostdev-info-addr.drive.target == 0) +hostdev-info-addr.drive.bus == bus +hostdev-info-addr.drive.target == target) return true; } @@ -5236,6 +5240,8 @@ virDomainDriveAddressIsUsedByHostdev(const virDomainDef *def, static bool virDomainSCSIDriveAddressIsUsed(const virDomainDef *def, unsigned int controller, +unsigned int bus, +unsigned int target, unsigned int unit) { /* In current implementation, the maximum unit number of a controller @@ -5245,9 +5251,10 @@ virDomainSCSIDriveAddressIsUsed(const virDomainDef *def, return true; if (virDomainDriveAddressIsUsedByDisk(def, VIR_DOMAIN_DISK_BUS_SCSI, - controller, unit) || -virDomainDriveAddressIsUsedByHostdev(def, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI, - controller, unit)) + controller, bus, target, unit) || +virDomainDriveAddressIsUsedByHostdev(def, + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI, + controller, bus, target, unit)) return true; return false; @@ -5262,7 +5269,8 @@ virDomainControllerSCSINextUnit(const virDomainDef *def, size_t i; for (i = 0; i max_unit; i++) { -if (!virDomainSCSIDriveAddressIsUsed(def, controller, i)) +/* Default to assigning addresses using bus = target = 0 */ +if (!virDomainSCSIDriveAddressIsUsed(def, controller, 0, 0, i)) return i; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] conf: Refactor virDomainDiskDefParseXML to pass vmdef
Rather than passing the def-seclabels and def-nseclabels, refactor the API to pass the entire domain definition. This will be used in a future patch as well. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e02cd49..6259d4a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6390,8 +6390,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, xmlNodePtr node, xmlXPathContextPtr ctxt, virHashTablePtr bootHash, - virSecurityLabelDefPtr* vmSeclabels, - int nvmSeclabels, + const virDomainDef *vmdef, unsigned int flags) { virDomainDiskDefPtr def; @@ -6930,8 +6929,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, ctxt-node = sourceNode; if (virSecurityDeviceLabelDefParseXML(def-src-seclabels, def-src-nseclabels, - vmSeclabels, - nvmSeclabels, + vmdef-seclabels, + vmdef-nseclabels, ctxt, flags) 0) goto error; @@ -12256,9 +12255,7 @@ virDomainDeviceDefParse(const char *xmlStr, switch ((virDomainDeviceType) dev-type) { case VIR_DOMAIN_DEVICE_DISK: if (!(dev-data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt, -NULL, def-seclabels, -def-nseclabels, -flags))) +NULL, def, flags))) goto error; break; case VIR_DOMAIN_DEVICE_LEASE: @@ -12400,9 +12397,7 @@ virDomainDiskDefSourceParse(const char *xmlStr, flags |= VIR_DOMAIN_DEF_PARSE_DISK_SOURCE; if (!(disk = virDomainDiskDefParseXML(xmlopt, node, ctxt, - NULL, def-seclabels, - def-nseclabels, - flags))) + NULL, def, flags))) goto cleanup; ret = disk-src; @@ -15418,8 +15413,7 @@ virDomainDefParseXML(xmlDocPtr xml, nodes[i], ctxt, bootHash, -def-seclabels, -def-nseclabels, +def, flags); if (!disk) goto error; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Additional SCSI generated device address checks
https://bugzilla.redhat.com/show_bug.cgi?id=1210587 These patches will resolve a couple issues with generation of the address type='drive' .../ for a SCSI disk and hostdev. The disk generation algorithm 'assumes' that when presented with target dev='sda'.../ that it can use controller=0 and unit=0 since sda would conceivably be the first device; however, a hostdev could attempt to assign itself to that address and it doesn't have a target device name, so it bypasses the virDomainDiskDefDstDuplicates checks that would normally 'catch' two disk's attempting to use the same name. Likewise, if a hostdev occupies an address and we attempt to hotplug a disk without providing an address, the address generation could attempt to place the disk on the already existing host device. John Ferlan (5): conf: Enforce SCSI hostdev address type conf: Add 'bus' and 'target' to SCSI address conflict checks conf: Add SCSI hostdev check for disk drive address already in use conf: Refactor virDomainDiskDefParseXML to pass vmdef conf: Check for hostdev conflicts when assign default disk address docs/formatdomain.html.in | 4 +- src/conf/domain_conf.c| 115 -- src/conf/domain_conf.h| 3 +- src/qemu/qemu_command.c | 4 +- src/vmx/vmx.c | 22 + src/vmx/vmx.h | 3 +- 6 files changed, 101 insertions(+), 50 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] yajl cleanups
Fixes 'make check' on RHEL 6, which I recently broke, then adds further improvements to the JSON parser. I'm tempted to push patch 1/3 as a build-breaker, but since the other two need review, I'll hold off and we can do all three as a series. Eric Blake (3): json: cope with older yajl semantics json: reject javascript comments json: reject trailing garbage src/util/virjson.c | 45 ++--- tests/jsontest.c | 5 - 2 files changed, 42 insertions(+), 8 deletions(-) -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/3] json: enhance parser test
We already enable the parser option to detect invalid UTF-8, but didn't test it. Also, JSON states that behavior of an object with a duplicated key is undefined; we chose to reject it, but were not testing it. With the enhanced tests in place, we can simplify yajl2 initialization by relying on parser defaults being sane. * src/util/virjson.c (virJSONValueFromString): Simplify. * tests/jsontest.c (mymain): Test more bad usage. Signed-off-by: Eric Blake ebl...@redhat.com --- src/util/virjson.c | 6 +- tests/jsontest.c | 2 ++ 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 3c6ed34..c123943 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1597,7 +1597,7 @@ virJSONValueFromString(const char *jsonstring) int rc; size_t len = strlen(jsonstring); # ifndef WITH_YAJL2 -yajl_parser_config cfg = { 0, 1 }; +yajl_parser_config cfg = { 0, 1 }; /* Match yajl 2 default behavior */ virJSONValuePtr tmp; # endif @@ -1605,10 +1605,6 @@ virJSONValueFromString(const char *jsonstring) # ifdef WITH_YAJL2 hand = yajl_alloc(parserCallbacks, NULL, parser); -if (hand) { -yajl_config(hand, yajl_allow_comments, 0); -yajl_config(hand, yajl_dont_validate_strings, 0); -} # else hand = yajl_alloc(parserCallbacks, cfg, NULL, parser); # endif diff --git a/tests/jsontest.c b/tests/jsontest.c index 97b9c0a..223f867 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -422,6 +422,7 @@ mymain(void) DO_TEST_PARSE_FAIL(trailing garbage, [] []); DO_TEST_PARSE_FAIL(list without array, 1, 1); DO_TEST_PARSE_FAIL(parser abuse, 1] [2); +DO_TEST_PARSE_FAIL(invalid UTF-8, \\x80\); DO_TEST_PARSE_FAIL(object with numeric keys, { 1:1, 2:1, 3:2 }); DO_TEST_PARSE_FAIL(unterminated object, { \1\:1, \2\:1, \3\:2); @@ -430,6 +431,7 @@ mymain(void) DO_TEST_PARSE_FAIL(array of an object with an array as a key, [ {[\key1\, \key2\]: \value\} ]); DO_TEST_PARSE_FAIL(object with unterminated key, { \key:7 }); +DO_TEST_PARSE_FAIL(duplicate key, { \a\: 1, \a\: 1 }); DO_TEST_FULL(lookup on array, Lookup, [ 1 ], NULL, false); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 00/10] new API virDomainBlockSetWriteThreshold
On 06/22/2015 05:06 PM, Eric Blake wrote: v2 was here: https://www.redhat.com/archives/libvir-list/2015-June/msg00591.html This series depends on my yajl/json cleanups: https://www.redhat.com/archives/libvir-list/2015-June/msg01098.html and can also be found here: git clone git://repo.or.cz/libvirt/ericb.git threshold http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/threshold Since then, I've rebased to master, addressed Peter's findings on v2, and tested that things actually work when combined with a single pending patch on qemu (I'll reply to this mail with the list id of that patch, where that patch in turn points to this series as justification). Depends on this qemu patch: https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05770.html -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 10/10] threshold: add write threshold setting in qemu
Time to wire up the new API to call into the QMP command for setting a write threshold. For now, the API only allows setting the threshold on the active layer. But I left TODOs in the series for places that need touching to find and support node names of backing files, so that we can use vda[1] notation to register for a threshold on the backing image during an active commit operation. * src/qemu/qemu_driver.c (qemuDomainBlockSetWriteThreshold): New function. * src/qemu/qemu_monitor.c (qemuMonitorBlockSetWriteThreshold): Likewise. * src/qemu/qemu_monitor.h (qemuMonitorBlockSetWriteThreshold): Likewise. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockSetWriteThreshold): Likewise. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockSetWriteThreshold): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_driver.c | 95 src/qemu/qemu_monitor.c | 13 ++ src/qemu/qemu_monitor.h | 5 +++ src/qemu/qemu_monitor_json.c | 30 ++ src/qemu/qemu_monitor_json.h | 3 ++ 5 files changed, 146 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 72e256b..732f102 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17906,6 +17906,100 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, return ret; } + +static int +qemuDomainBlockSetWriteThreshold(virDomainPtr dom, + const char *path, + unsigned long long threshold, + unsigned int flags) +{ +virQEMUDriverPtr driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +virDomainDiskDefPtr disk = NULL; +virCapsPtr caps = NULL; +virQEMUDriverConfigPtr cfg = NULL; +const char *node = NULL; +int ret = -1; + +virCheckFlags(VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PROPORTION, -1); + +if (!(vm = qemuDomObjFromDomain(dom))) +return -1; +priv = vm-privateData; + +if (virDomainBlockSetWriteThresholdEnsureACL(dom-conn, vm-def) 0) +goto cleanup; + +if (!(caps = virQEMUDriverGetCapabilities(driver, false))) +goto cleanup; +if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCK_WRITE_THRESHOLD) || +!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_AUTO_NODE_NAMES)) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(block write threshold not supported by this + qemu binary)); +goto cleanup; +} + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(domain is not running)); +goto endjob; +} + +if (!(disk = virDomainDiskByName(vm-def, path, false))) { +virReportError(VIR_ERR_INVALID_ARG, + _(invalid path %s not assigned to domain), path); +goto endjob; +} + +if (!(node = qemuDomainDiskGetAllocationNode(driver, vm, disk))) { +virReportError(VIR_ERR_INVALID_ARG, + _(cannot track threshold events on disk '%s'), + path); +goto endjob; +} + +cfg = virQEMUDriverGetConfig(driver); +if (qemuStorageLimitsRefresh(driver, cfg, vm, disk-src) 0) +goto endjob; + +if (flags VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PROPORTION) { +/* Caller already sanitized max value to 100. Use of + * floating point intermediary reduces (but does not + * eliminate) rounding error, but since we already document a + * granularity of parts per million, it shouldn't matter too + * much if the answer is a few bytes off. */ +threshold *= disk-src-physical / 1e6; +} else { +if (threshold disk-src-physical) { +virReportError(VIR_ERR_INVALID_ARG, + _(threshold %llu exceeds disk size %llu), + threshold, disk-src-physical); +goto endjob; +} +} + +qemuDomainObjEnterMonitor(driver, vm); +ret = qemuMonitorBlockSetWriteThreshold(priv-mon, node, threshold); +if (qemuDomainObjExitMonitor(driver, vm) 0) +ret = -1; + + + endjob: +qemuDomainObjEndJob(driver, vm); + + cleanup: +virDomainObjEndAPI(vm); +virObjectUnref(caps); +virObjectUnref(cfg); +return ret; +} + + static int qemuDomainGetDiskErrors(virDomainPtr dom, virDomainDiskErrorPtr errors, @@ -20037,6 +20131,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .nodeSuspendForDuration = qemuNodeSuspendForDuration, /* 0.9.8 */ .domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */ .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */ +.domainBlockSetWriteThreshold = qemuDomainBlockSetWriteThreshold, /* 1.3.0 */
[libvirt] [PATCHv3 08/10] threshold: scrape threshold data from QMP
Expose threshold information by collecting the information from QMP commands. qemu 2.3 has a way to get threshold information on all elements of a block chain, but only when node names are used - what's worse, the threshold information is only provided by 'query-named-block-nodes', but the mapping between devices and nodes is only provided by 'query-blockstats'. Rather than manage node names ourselves, we rely on qemu 2.4 doing auto naming; then when collecting the stats, we make a pass through both query functions. I chose to go with the naive O(n^2) mapping algorithm; if it turns out to be slow in practice on a guest with lots of nodes, we can enhance it via a sorted list or hash table lookup. * src/qemu/qemu_monitor.h (qemuMonitorBlockStatsUpdateThreshold): New prototype. * src/qemu/qemu_monitor.c (qemuMonitorBlockStatsUpdateThreshold): New function. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockStatsUpdateThreshold): New prototype. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONDevGetBlockExtent): Populate node name. (qemuMonitorJSONBlockStatsUpdateThreshold) (qemuMonitorJSONBlockStatsUpdateOneThreshold): Use it to populate threshold data. (qemuMonitorJSONGetOneBlockStatsInfo) (qemuMonitorJSONGetBlockExtent): Update callers. * src/qemu/qemu_driver.c (qemuDomainGetStatsOneBlock): Expose threshold data. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_driver.c | 12 - src/qemu/qemu_monitor.c | 13 ++ src/qemu/qemu_monitor.h | 4 ++ src/qemu/qemu_monitor_json.c | 102 ++- src/qemu/qemu_monitor_json.h | 2 + 5 files changed, 121 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 25bc76d..72e256b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19282,6 +19282,12 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, fl.times, entry-flush_total_times); +/* TODO: Until we can set thresholds on backing elements, we only + * report the threshold on the active layer. */ +if (!backing_idx) +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + write-threshold, entry-write_threshold); + QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, allocation, entry-wr_highest_offset); @@ -19323,9 +19329,13 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, dom); rc = qemuMonitorGetAllBlockStatsInfo(priv-mon, stats, visitBacking); -if (rc = 0) +if (rc = 0) { ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv-mon, stats, visitBacking)); +if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_AUTO_NODE_NAMES)) +ignore_value(qemuMonitorBlockStatsUpdateThreshold(priv-mon, + stats)); +} if (qemuDomainObjExitMonitor(driver, dom) 0) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 86dc4be..a3ad740 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1838,6 +1838,19 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, } +/* Updates stats to fill write threshold of the image */ +int +qemuMonitorBlockStatsUpdateThreshold(qemuMonitorPtr mon, + virHashTablePtr stats) +{ +VIR_DEBUG(stats=%p, stats); + +QEMU_CHECK_MONITOR_JSON(mon); + +return qemuMonitorJSONBlockStatsUpdateThreshold(mon, stats); +} + + int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c0ea2ee..541f774 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -395,6 +395,10 @@ int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, bool backingChain) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuMonitorBlockStatsUpdateThreshold(qemuMonitorPtr mon, + virHashTablePtr stats) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 32b2719..885b6f4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1668,9 +1668,13 @@ typedef enum { } qemuMonitorBlockExtentError; +/* Get the highest written extent. Additionally, if NODE is not null, + * and a node name is associated with the extent, then return that + * name; but failure to get a node name
[libvirt] [PATCHv3 07/10] threshold: track an allocation node name for a storage source
Set up functions to make it easy to map between libvirt disk names and qemu node names. Although we won't expose the information in XML, it is still nicer to cache the information than to have to grab a job lock, so that we are less likely to need to re-query the monitor when dealing with a qemu monitor event. We need the information in two places: one in the hash table used to collect QMP stats (as the QMP design requires using 'query-blockstats' to learn node names, followed by 'query-named-block-nodes' to learn statistics about those nodes), and the other in virStorageSourcePtr (the disk information tracked by libvirt). Making sure this doesn't leak memory was interesting; in particular, in qemu_driver.c:qemUDomainBlocksStatsGather(), I made sure that the returned result does not set an allocation name, so that the callers of that function can continue to use VIR_FREE. * src/util/virstoragefile.h (_virStorageSource): Add a field. * src/util/virstoragefile.c (virStorageSourceBackingStoreClear): Clear it. (virStorageSourceCopy): Document that it is not deep-copied. * src/qemu/qemu_domain.c (qemuDomainDiskGetAllocationNode) (qemuDomainDiskResolveAllocationNode): New functions, to use the field. (qemuDomainDiskSupportsAllocationNode): New helper. * src/qemu/qemu_domain.h: Declare new functions. * src/qemu/qemu_monitor.h (_qemuBlockStats): Add fields. (qemuMonitorCleanBlockStats): New declaration. * src/qemu/qemu_monitor.c (qemuMonitorCleanBlockStats): New function. (qemuMonitorGetAllBlockStatsInfo): Use it to avoid leak. * src/qemu/qemu_migration.c (qemuMigrationCookieAddNBD): Likewise. * src/qemu/qemu_driver.c (qemuDomainBlocksStatsGather): Don't leak memory. * tests/qemumonitortest.c (testBlockInfoData): Update test. --- src/qemu/qemu_domain.c| 105 ++ src/qemu/qemu_domain.h| 10 - src/qemu/qemu_driver.c| 1 + src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 12 +- src/qemu/qemu_monitor.h | 4 ++ src/util/virstoragefile.c | 4 +- src/util/virstoragefile.h | 3 +- tests/qemumonitortest.c | 13 +++--- 9 files changed, 143 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6213fd9..d3ce7db 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2906,6 +2906,111 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, } +static bool +qemuDomainDiskSupportsAllocationNode(virDomainDiskDefPtr disk) +{ +/* For now, only qcow2 images backed by block devices are supported. */ +if (disk-src-type != VIR_STORAGE_TYPE_BLOCK) +return false; +if (disk-src-format != VIR_STORAGE_FILE_QCOW2) +return false; +return true; +} + + +/* Determine the node name that qemu associates with allocation + * tracking. Cache the value to minimize queries. Return NULL if the + * storage source is not a qcow2 formatted block device (as those are + * the only devices where live allocation tracking is useful), or if + * qemu cannot determine a node name. Requires a live domain, and + * assumes that the job lock is held, as this may require monitor + * interaction. */ +const char * +qemuDomainDiskGetAllocationNode(virQEMUDriverPtr driver, +virDomainObjPtr vm, +virDomainDiskDefPtr disk) +{ +virStorageSourcePtr src = disk-src; +char *name = NULL; +qemuDomainObjPrivatePtr priv = vm-privateData; + +if (!qemuDomainDiskSupportsAllocationNode(disk)) +return NULL; + +if (src-allocation_node) +goto cleanup; + +qemuDomainObjEnterMonitor(driver, vm); +/* TODO: allow lookup of node name of backing images */ +name = qemuMonitorNodeNameLookup(priv-mon, disk-info.alias); +if (qemuDomainObjExitMonitor(driver, vm) 0) +goto cleanup; + +src-allocation_node = name; +name = NULL; + + cleanup: +VIR_FREE(name); +return src-allocation_node; +} + + +/* Determine the disk name that matches a node name returned by qemu + * in a threshold event. In the common case, no job is needed: the + * node name will be cached from when the threshold was + * registered. But if the cache is lost (such as over a libvirtd + * restart), JOB_OKAY states whether it is safe to rebuild the cache + * (requires a live domain and monitor interaction), instead of + * failing. */ +virDomainDiskDefPtr +qemuDomainDiskResolveAllocationNode(virQEMUDriverPtr driver, +virDomainObjPtr vm, +const char *node, +bool job_okay) +{ +size_t i; +virDomainDiskDefPtr disk; +virDomainDiskDefPtr ret = NULL; +char *name = NULL; +qemuDomainObjPrivatePtr priv = vm-privateData; + +/* First pass - see if the node name is cached. */ +for (i = 0; i vm-def-ndisks; i++) { +disk = vm-def-disks[i]; +if
[libvirt] [PATCHv3 02/10] threshold: expose new API in virsh
Add a new 'virsh domblkthreshold' command to use the new API. The main use is an obvious mapping to the new API: virsh domblkthreshold $dom $disk 1000 # 10M bytes or virsh domblkthreshold $dom $disk 101000 --proportion # 10.1% but I also wanted to be lazy at computing parts per million, so I allow: virsh domblkthreshold $dom $disk --percentage 10.1% # as before * tools/virsh.pod (domblkthreshold): Document it. * tools/virsh-domain-monitor.c (cmdDomblkthreshold): New function. (domMonitoringCmds): Register it. Signed-off-by: Eric Blake ebl...@redhat.com --- tools/virsh-domain-monitor.c | 108 +++ tools/virsh.pod | 25 ++ 2 files changed, 133 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 1d4dc25..5a0287f 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -571,6 +571,108 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) } /* + * domblkthreshold command + */ +static const vshCmdInfo info_domblkthreshold[] = { +{.name = help, + .data = N_(set domain block device write thresholds) +}, +{.name = desc, + .data = N_(Set a threshold to get a one-shot event if block +allocation exceeds that size) +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_domblkthreshold[] = { +{.name = domain, + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_(domain name, id or uuid), +}, +{.name = device, + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_(block device), +}, +{.name = threshold, + .type = VSH_OT_INT, + .help = N_(new threshold, or 0 to disable), +}, +{.name = proportion, + .type = VSH_OT_BOOL, + .help = N_(threshold is in parts-per-million instead of bytes), +}, +{.name = percentage, + .type = VSH_OT_STRING, /* floating point doesn't have an option type */ + .flags = VSH_OFLAG_REQ_OPT, + .help = N_(determine threshold from a percentage), +}, +{.name = NULL} +}; + +static bool +cmdDomblkthreshold(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom; +bool ret = false; +const char *device = NULL; +unsigned long long threshold; +bool proportion = vshCommandOptBool(cmd, proportion); +unsigned int flags = 0; +const char *percentage = NULL; + +VSH_EXCLUSIVE_OPTIONS(threshold, percentage); +VSH_EXCLUSIVE_OPTIONS(proportion, percentage); + +if (vshCommandOptStringReq(ctl, cmd, percentage, percentage) 0) +return false; +if (percentage) { +char *end; +double raw; + +if (virStrToDouble(percentage, end, raw) 0 || +end[*end == '%'] || raw 0.0 || raw 100.0) { +vshError(ctl, _(unable to parse '%s' as percentage), percentage); +return false; +} +threshold = raw * 1; +proportion = true; +} else if (!vshCommandOptBool(cmd, threshold)) { +vshError(ctl, %s, + _(either --threshold or --percentage is required)); +return false; +} else if (vshCommandOptULongLong(ctl, cmd, threshold, threshold) 0) { +return false; +} + +if (proportion) +flags |= VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PROPORTION; + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return false; + +if (vshCommandOptStringReq(ctl, cmd, device, device) 0) +goto cleanup; + +if (virDomainBlockSetWriteThreshold(dom, device, threshold, flags) 0) +goto cleanup; + +if (proportion) +vshPrint(ctl, _(threshold of %s set to %llu.%04llu%%\n), + device, threshold / 1, threshold % 1); +else +vshPrint(ctl, _(threshold of %s set to %llu bytes\n), + device, threshold); + +ret = true; + + cleanup: +virDomainFree(dom); +return ret; +} + +/* * domiflist command */ static const vshCmdInfo info_domiflist[] = { @@ -2358,6 +2460,12 @@ const vshCmdDef domMonitoringCmds[] = { .info = info_domblkstat, .flags = 0 }, +{.name = domblkthreshold, + .handler = cmdDomblkthreshold, + .opts = opts_domblkthreshold, + .info = info_domblkthreshold, + .flags = 0 +}, {.name = domcontrol, .handler = cmdDomControl, .opts = opts_domcontrol, diff --git a/tools/virsh.pod b/tools/virsh.pod index 600ea42..1b67a72 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -843,6 +843,31 @@ that require a block device name (such as Idomblkinfo or Isnapshot-create for disk snapshots) will accept either target or unique source names printed by this command. +=item Bdomblkthreshold Idomain Iblock-device { Ithreshold +[I--proportion] | I--percentage Ivalue } + +Set the write threshold for a block device of a domain. A +Iblock-device corresponds to a unique target name (target +dev='name'/) or source file (source file='name'/) for one
[libvirt] [PATCHv3 05/10] threshold: add qemu capability bits
Track whether qemu is new enough to do block thresholds on the active layer (feature added in qemu 2.3). The plan is that even if qemu is too old, attempts to register a threshold value will return failure; but the event handler callback can still be registered successfuly (and will just never fire). Thresholds only work if you use named nodes. But overhauling libvirt to track named nodes is a pain; particularly since a single qcow2 libvirt disk element has multiple qemu nodes (one for the qcow2 protocol, that controls what the guest sees, and one for the underlying file, that controls what is done on the host); the threshold information we are interested in is associated with the host view, but that is NOT the node at the active layer of a block device. So while we may eventually want to track node names in libvirt XML and have control over the names of (some) nodes used by qemu, there is no guarantee that we want to do it for all nodes. For now, it is easier to just assume that qemu will supply a generated node name when we don't (a feature added in qemu 2.4), and the rest of this series will merely cache generated node names (re-querying it from qemu as needed after a libvirtd restart) rather than trying to control a fixed name. The ability to set thresholds and the ability to auto name nodes are indepenent enough to be backported separately in downstream qemu, so we have to use two capability bits, and will require both to be set before allowing a write threshold registration. If my assumptions during probing are ever broken (for example, if fdset 0 is no longer wired to read-only /dev/null, or if a named node already exists prior to our point of probing), the code could be made more robust by using the 'null-co' driver instead of 'file', and then by iterating through the array to ensure one of the nodes is visiting the just-registered fdset file. But this patch works for now. I _hate_ the way qemucapabilitiestests works - it is NOT easy to debug how to fix failures in that test. But I don't have time to revamp it today (ideally, the .replies file should include as a comment the command that was associated with a given reply, so that injecting new commands in the code can more easily match that to the .replies file. Furthermore, the replies should not be so sensitive to changing id fields). * src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCK_WRITE_THRESHOLD) (QEMU_CAPS_AUTO_NODE_NAMES): New bits * src/qemu/qemu_capabilities.c (virQEMUCapsCommands): Enable them... (virQEMUCapsProbeQMPCommands): ...but only if they actually work. * src/qemu/qemu_monitor.c (qemuMonitorSupportsNodeNames): New function. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONSupportsNodeNames): Likewise. * src/qemu/qemu_monitor.h (qemuMonitorSupportsNodeNames): New prototype. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONSupportsNodeNames): Likewise. * tests/qemucapabilitiesdata/caps_2.1.1-1.replies: Update test. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_capabilities.c| 12 +- src/qemu/qemu_capabilities.h| 4 +- src/qemu/qemu_monitor.c | 12 + src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c| 65 src/qemu/qemu_monitor_json.h| 3 + tests/qemucapabilitiesdata/caps_2.1.1-1.replies | 470 7 files changed, 336 insertions(+), 231 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e7002a3..af597e3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1,7 +1,7 @@ /* * qemu_capabilities.c: QEMU capabilities generation * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -288,6 +288,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, vhost-user-multiqueue, /* 190 */ arm-virt-pci, + block-write-threshold, + auto-node-names, ); @@ -1499,6 +1501,8 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { nbd-server-start, QEMU_CAPS_NBD_SERVER }, { change-backing-file, QEMU_CAPS_CHANGE_BACKING_FILE }, { rtc-reset-reinjection, QEMU_CAPS_RTC_RESET_REINJECTION }, +{ block-set-write-threshold, QEMU_CAPS_BLOCK_WRITE_THRESHOLD }, +{ query-named-block-nodes, QEMU_CAPS_AUTO_NODE_NAMES }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { @@ -2359,6 +2363,12 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, qemuMonitorSupportsActiveCommit(mon)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT); +/* Qemu 2.3 did not automatically set node names, so querying node + * information is useless if we don't supply them. */ +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_AUTO_NODE_NAMES) +
[libvirt] [PATCHv3 01/10] threshold: new API virDomainBlockSetWriteThreshold
qemu 2.3 added a new QMP command block-set-write-threshold, which allows callers to get an interrupt when a file hits a write threshold, rather than the current approach of repeatedly polling for file allocation. This patch prepares the API for callers to register to receive the event, as well as a way to query the threshold via virDomainListGetStats(). The event is one-shot in qemu - a guest must re-register a new threshold after each time it triggers. However, the virConnectDomainEventRegisterAny() call does not allow parameterization, so callers must use a pair of APIs - one to register the callback (one-time call) that will be used each time a threshold triggers for any guest disk, and another to repeatedly set the desired threshold (must be called each time a threshold should be changed). Note that the threshold can either be registered by a byte offset, or by a parts-per-million proportion (a value between 0 and 100, scaled to the disk size). But the value is always reported as a byte offset, even when registered as a proportion. I also considered having the setup parameter be a double, to allow a finer resolution rather than fixed-point proportion; but that much resolution is probably not necessary (for a 100G disk, the resulting 100k granularity is pretty much in the noise). To make the patch series more digestible, this patch intentionally omits remote support, by using a couple of placeholders at a point where the compiler forces the addition of a case label within a switch statement. * include/libvirt/libvirt-domain.h (virDomainBlockSetWriteThreshold): New API. (virConnectDomainEventWriteThresholdCallback): New event. * src/libvirt_public.syms (LIBVIRT_1.3.0): Export it. * src/libvirt-domain.c (virDomainBlockSetWriteThreshold): New API. (virConnectGetAllDomainStats): New stat. * src/driver-hypervisor.h (virDrvDomainBlockSetWriteThreshold): New hypervisor entry point. * tools/virsh-domain.c (vshEventWriteThresholdPrint): Print new event. * tools/virsh.pod (domstats): Document new stat. * daemon/remote.c (domainEventCallbacks): Add stub. * src/conf/domain_event.c (virDomainEventDispatchDefaultFunc): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- daemon/remote.c | 2 + include/libvirt/libvirt-domain.h | 53 ++ src/conf/domain_event.c | 4 +- src/driver-hypervisor.h | 7 +++ src/libvirt-domain.c | 98 src/libvirt_public.syms | 1 + tools/virsh-domain.c | 24 ++ tools/virsh.pod | 1 + 8 files changed, 189 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index e9e2dca..283ece2 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1102,6 +1102,8 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventAgentLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded), +/* TODO: Implement RPC support for this */ +VIR_DOMAIN_EVENT_CALLBACK(NULL), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7564c20..ca2f929 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1306,6 +1306,18 @@ int virDomainBlockStatsFlags (virDomainPtr dom, virTypedParameterPtr params, int *nparams, unsigned int flags); + +typedef enum { +/* threshold is a parts-per-million proportion of the image size + * rather than byte limit */ +VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PROPORTION = (1 0), +} virDomainBlockSetWriteThresholdFlags; + +int virDomainBlockSetWriteThreshold(virDomainPtr dom, +const char *disk, +unsigned long long threshold, +unsigned int flags); + int virDomainInterfaceStats (virDomainPtr dom, const char *path, virDomainInterfaceStatsPtr stats, @@ -3255,6 +3267,46 @@ typedef void (*virConnectDomainEventDeviceAddedCallback)(virConnectPtr conn, void *opaque); /** + * virConnectDomainEventWriteThresholdCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @devAlias: device alias + * @path: a local path name of the host resource, or NULL if not available + * @threshold: threshold that was exceeded, in bytes + * @length: length beyond @threshold that was involved in the triggering + * write, or 0
[libvirt] [PATCHv3 04/10] threshold: wire up threshold event in RPC
Pass the new virDomainBlockSetWriteThreshold() API and WRITE_THRESHOLD event across RPC. The event takes the bulk of the patch, but everything here is pretty mechanical and copies patterns from existing code. Yes, it is intentional that registration takes a single mandatory string (which can be either vda or path notation), while the reported event has two strings (one mandatory, for vda; the other optional for when path is available). I debated about splitting the event separately from the API for setting the threshold, in case we ever want to support setting a write threshold by overloading an existing API such as virDomainSetBlockIoTune(); but consensus on the design was that overloading existing API was not appropriate, and that no one is expecting to use this functionality without also being prepared to require the .so bump. * daemon/remote.c (remoteRelayDomainEventWriteThreshold): New function. * src/remote/remote_driver.c (remoteDomainBuildEventCallbackWriteThreshold): Likewise. * src/remote/remote_protocol.x (remote_domain_event_callback_write_threshold_msg) (remote_domain_block_set_write_threshold_args): New structs. * src/remote_protocol-structs: Update. Signed-off-by: Eric Blake ebl...@redhat.com --- daemon/remote.c | 53 ++-- src/remote/remote_driver.c | 35 + src/remote/remote_protocol.x | 30 - src/remote_protocol-structs | 16 + 4 files changed, 131 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 283ece2..457e220 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1079,6 +1079,56 @@ remoteRelayDomainEventDeviceAdded(virConnectPtr conn, } +static int +remoteRelayDomainEventWriteThreshold(virConnectPtr conn, + virDomainPtr dom, + const char *disk, + const char *path, + unsigned long long threshold, + unsigned long long length, + void *opaque) +{ +daemonClientEventCallbackPtr callback = opaque; +remote_domain_event_callback_write_threshold_msg data; +char **path_p = NULL; + +if (callback-callbackID 0 || +!remoteRelayDomainEventCheckACL(callback-client, conn, dom)) +return -1; + +VIR_DEBUG(Relaying domain write threshold event %s %d %s (%s) %llu %llu, + callback %d, dom-name, dom-id, + disk, NULLSTR(path), threshold, length, callback-callbackID); + +/* build return data */ +memset(data, 0, sizeof(data)); + +if (VIR_STRDUP(data.disk, disk) 0) +goto error; +if (path +((VIR_ALLOC(path_p) 0) || + VIR_STRDUP(*path_p, path) 0)) +goto error; + +make_nonnull_domain(data.dom, dom); +data.callbackID = callback-callbackID; +data.path = path_p; +data.threshold = threshold; +data.length = length; + +remoteDispatchObjectEventSend(callback-client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_WRITE_THRESHOLD, + (xdrproc_t)xdr_remote_domain_event_callback_write_threshold_msg, + data); + +return 0; + error: +VIR_FREE(data.disk); +VIR_FREE(data.path); +VIR_FREE(path_p); +return -1; +} + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { @@ -1102,8 +1152,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventAgentLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded), -/* TODO: Implement RPC support for this */ -VIR_DOMAIN_EVENT_CALLBACK(NULL), +VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventWriteThreshold), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 273799b..f0b1941 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -342,6 +342,12 @@ remoteDomainBuildEventCallbackAgentLifecycle(virNetClientProgramPtr prog, void *evdata, void *opaque); static void +remoteDomainBuildEventCallbackWriteThreshold(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); + + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque); @@ -504,6 +510,10 @@ static virNetClientProgramEvent remoteEvents[] = {
[libvirt] [PATCHv3 09/10] threshold: add threshold event handling in qemu
With this patch, block write threshold events delivered by qemu are converted into libvirt events. This patch takes the easy road, and only reports events if the node name is still cached by libvirtd (true in the common case when libvirtd is not restarted, since you can't get an event if you didn't register a threshold). A followup patch will be needed to properly handle grabbing the job lock and getting the node name when the cache lookup fails, using code similar to how we update domain XML after a block job completes. But for getting the initial API in place, I figured this was a good enough start. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONHandleBlockWriteThreshold): New function. * src/qemu/qemu_monitor.c (qemuMonitorEmitBlockThreshold): Likewise. * src/qemu/qemu_monitor.h (qemuMonitorEmitBlockThreshold): Likewise. * src/qemu/qemu_process.c (qemuProcessHandleBlockThreshold): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor.c | 14 ++ src/qemu/qemu_monitor.h | 11 +++ src/qemu/qemu_monitor_json.c | 31 +++ src/qemu/qemu_process.c | 44 4 files changed, 100 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a3ad740..c4b6073 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1433,6 +1433,20 @@ qemuMonitorEmitBlockJob(qemuMonitorPtr mon, } +int qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, + const char *node, + unsigned long long threshold, + unsigned long long length) +{ +int ret = -1; +VIR_DEBUG(mon=%p, mon); + +QEMU_MONITOR_CALLBACK(mon, ret, domainBlockThreshold, mon-vm, + node, threshold, length); +return ret; +} + + int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, unsigned long long actual) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 541f774..8c84cdb 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -146,6 +146,12 @@ typedef int (*qemuMonitorDomainBlockJobCallback)(qemuMonitorPtr mon, int type, int status, void *opaque); +typedef int (*qemuMonitorDomainBlockThresholdCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *node, + unsigned long long threshold, + unsigned long long length, + void *opaque); typedef int (*qemuMonitorDomainTrayChangeCallback)(qemuMonitorPtr mon, virDomainObjPtr vm, const char *devAlias, @@ -204,6 +210,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainIOErrorCallback domainIOError; qemuMonitorDomainGraphicsCallback domainGraphics; qemuMonitorDomainBlockJobCallback domainBlockJob; +qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; qemuMonitorDomainTrayChangeCallback domainTrayChange; qemuMonitorDomainPMWakeupCallback domainPMWakeup; qemuMonitorDomainPMSuspendCallback domainPMSuspend; @@ -301,6 +308,10 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, const char *diskAlias, int type, int status); +int qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, + const char *node, + unsigned long long threshold, + unsigned long long length); int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, unsigned long long actual); int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 885b6f4..46383d7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -77,6 +77,8 @@ static void qemuMonitorJSONHandlePMSuspend(qemuMonitorPtr mon, virJSONValuePtr d static void qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJobReady(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleBlockWriteThreshold(qemuMonitorPtr mon, + virJSONValuePtr data); static void qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, virJSONValuePtr data); static
[libvirt] [PATCHv3 06/10] threshold: learn a node name for a given qcow2 disk
Implement the QMP side of asking qemu what node name it assigned to an arbitrary top-level device node. Assumes that the caller will have already validated that the device is qcow2 backed by a block device, and that qemu auto-assigns node names. * src/qemu/qemu_monitor.h (qemuMonitorNodeNameLookup): New function. * src/qemu/qemu_monitor.c (qemuMonitorNodeNameLookup): Likewise. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONNodeNameLookup): Likewise. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONNodeNameLookup): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor.c | 15 - src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 77 src/qemu/qemu_monitor_json.h | 3 ++ 4 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2e9e2de..5612491 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3094,7 +3094,7 @@ qemuMonitorSupportsNodeNames(qemuMonitorPtr mon) } -/* Determine the name that qemu is using for tracking the backing +/* Determine the path name that qemu is using for tracking the backing * element TARGET within the chain starting at TOP. */ char * qemuMonitorDiskNameLookup(qemuMonitorPtr mon, @@ -3108,6 +3108,19 @@ qemuMonitorDiskNameLookup(qemuMonitorPtr mon, } +/* Determine the node name that qemu is using for tracking the raw + * file of the qcow2 protocol at DEVICE. */ +char * +qemuMonitorNodeNameLookup(qemuMonitorPtr mon, + const char *device) +{ +/* TODO - change signature to allow backing file lookups */ +QEMU_CHECK_MONITOR_JSON_NULL(mon); + +return qemuMonitorJSONNodeNameLookup(mon, device); +} + + /* Use the block-job-complete monitor command to pivot a block copy job. */ int qemuMonitorDrivePivot(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index e198c06..826835b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -745,6 +745,9 @@ char *qemuMonitorDiskNameLookup(qemuMonitorPtr mon, virStorageSourcePtr top, virStorageSourcePtr target) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +char *qemuMonitorNodeNameLookup(qemuMonitorPtr mon, +const char *device) +ATTRIBUTE_NONNULL(2); int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e4701aa..32b2719 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3835,6 +3835,8 @@ qemuMonitorJSONDiskNameLookupOne(virJSONValuePtr image, } +/* Look up the image name that qemu is using for a member in a backing + * chain. */ char * qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon, const char *device, @@ -3900,6 +3902,81 @@ qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon, } +/* Look up the node name of the file underneath a qcow2 image. */ +char * +qemuMonitorJSONNodeNameLookup(qemuMonitorPtr mon, + const char *device) +{ +char *ret = NULL; +virJSONValuePtr cmd = NULL; +virJSONValuePtr reply = NULL; +virJSONValuePtr devices; +size_t i; + +cmd = qemuMonitorJSONMakeCommand(query-blockstats, NULL); +if (!cmd) +return NULL; +if (qemuMonitorJSONCommand(mon, cmd, reply) 0) +goto cleanup; + +if (!(devices = virJSONValueObjectGetArray(reply, return))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(blockstats reply was missing device list)); +goto cleanup; +} + +for (i = 0; i virJSONValueArraySize(devices); i++) { +virJSONValuePtr dev = virJSONValueArrayGet(devices, i); +virJSONValuePtr parent; +const char *thisdev; +const char *node; + +if (!dev || dev-type != VIR_JSON_TYPE_OBJECT) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(blockstats entry was not in expected format)); +goto cleanup; +} + +if (!(thisdev = virJSONValueObjectGetString(dev, device))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(blockstats entry was not in expected format)); +goto cleanup; +} + +if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) +thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); +if (STRNEQ(thisdev, device)) +continue; + +if (!(parent = virJSONValueObjectGetObject(dev, parent))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(blockstats device %s missing parent node), + device); +goto cleanup; +} + +if (!(node = virJSONValueObjectGetString(parent,
[libvirt] [PATCHv3 03/10] threshold: new event object for tracking write threshold
Common code for tracking a write threshold event from creation to delivery through a callback. * src/conf/domain_event.c (_virDomainEventWriteThreshold): New struct. (virDomainEventsOnceInit, virDomainEventWriteThresholdDispose) (virDomainEventWriteThresholdNew) (virDomainEventWriteThresholdNewFromObj) (virDomainEventWriteThresholdNewFromDom) (virDomainEventDispatchDefaultFunc): Wire it up. * src/conf/domain_event.h: Expose new functions. * src/libvirt-private.syms (domain_event.h): Export them. Signed-off-by: Eric Blake ebl...@redhat.com --- src/conf/domain_event.c | 99 +++- src/conf/domain_event.h | 16 +++- src/libvirt_private.syms | 2 + 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index c43799f..400cfa0 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -56,6 +56,7 @@ static virClassPtr virDomainQemuMonitorEventClass; static virClassPtr virDomainEventTunableClass; static virClassPtr virDomainEventAgentLifecycleClass; static virClassPtr virDomainEventDeviceAddedClass; +static virClassPtr virDomainEventWriteThresholdClass; static void virDomainEventDispose(void *obj); @@ -74,6 +75,7 @@ static void virDomainQemuMonitorEventDispose(void *obj); static void virDomainEventTunableDispose(void *obj); static void virDomainEventAgentLifecycleDispose(void *obj); static void virDomainEventDeviceAddedDispose(void *obj); +static void virDomainEventWriteThresholdDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -236,6 +238,17 @@ struct _virDomainEventAgentLifecycle { typedef struct _virDomainEventAgentLifecycle virDomainEventAgentLifecycle; typedef virDomainEventAgentLifecycle *virDomainEventAgentLifecyclePtr; +struct _virDomainEventWriteThreshold { +virDomainEvent parent; + +char *disk; +char *path; +unsigned long long threshold; +unsigned long long length; +}; +typedef struct _virDomainEventWriteThreshold virDomainEventWriteThreshold; +typedef virDomainEventWriteThreshold *virDomainEventWriteThresholdPtr; + static int virDomainEventsOnceInit(void) @@ -336,6 +349,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainEventAgentLifecycle), virDomainEventAgentLifecycleDispose))) return -1; +if (!(virDomainEventWriteThresholdClass = + virClassNew(virDomainEventClass, + virDomainEventWriteThreshold, + sizeof(virDomainEventWriteThreshold), + virDomainEventWriteThresholdDispose))) +return -1; return 0; } @@ -496,6 +515,16 @@ virDomainEventAgentLifecycleDispose(void *obj) VIR_DEBUG(obj=%p, event); }; +static void +virDomainEventWriteThresholdDispose(void *obj) +{ +virDomainEventWriteThresholdPtr event = obj; +VIR_DEBUG(obj=%p, event); + +VIR_FREE(event-disk); +VIR_FREE(event-path); +}; + static void * virDomainEventNew(virClassPtr klass, @@ -1390,6 +1419,62 @@ virDomainEventTunableNewFromDom(virDomainPtr dom, nparams); } +static virObjectEventPtr +virDomainEventWriteThresholdNew(int id, +const char *name, +unsigned char *uuid, +const char *disk, +const char *path, +unsigned long long threshold, +unsigned long long length) +{ +virDomainEventWriteThresholdPtr ev; + +if (virDomainEventsInitialize() 0) +return NULL; + +if (!(ev = virDomainEventNew(virDomainEventWriteThresholdClass, + VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD, + id, name, uuid))) +return NULL; + +if (VIR_STRDUP(ev-disk, disk) 0) +goto error; +if (VIR_STRDUP(ev-path, path) 0) +goto error; +ev-threshold = threshold; +ev-length = length; + +return (virObjectEventPtr)ev; + + error: +virObjectUnref(ev); +return NULL; +} + +virObjectEventPtr +virDomainEventWriteThresholdNewFromObj(virDomainObjPtr obj, + const char *disk, + const char *path, + unsigned long long threshold, + unsigned long long length) +{ +return virDomainEventWriteThresholdNew(obj-def-id, obj-def-name, + obj-def-uuid, disk, path, + threshold, length); +} + +virObjectEventPtr +virDomainEventWriteThresholdNewFromDom(virDomainPtr dom, + const char *disk, + const char *path, + unsigned long long threshold, +
[libvirt] [PATCHv3 00/10] new API virDomainBlockSetWriteThreshold
v2 was here: https://www.redhat.com/archives/libvir-list/2015-June/msg00591.html This series depends on my yajl/json cleanups: https://www.redhat.com/archives/libvir-list/2015-June/msg01098.html and can also be found here: git clone git://repo.or.cz/libvirt/ericb.git threshold http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/threshold Since then, I've rebased to master, addressed Peter's findings on v2, and tested that things actually work when combined with a single pending patch on qemu (I'll reply to this mail with the list id of that patch, where that patch in turn points to this series as justification). Here's a transcript of how I tested: I modified my 'f20' domain to include: devices emulator/home/eblake/qemu/x86_64-softmmu/qemu-system-x86_64/emulator disk type='block' device='disk' driver name='qemu' type='qcow2'/ source dev='/dev/sda6'/ target dev='vda' bus='virtio'/ /disk then created a raw partition wrapper around the usual disk image: # qemu-img create -f qcow2 -o backing_file=/var/lib/libvirt/images/f20.img,backing_fmt=qcow2 /dev/sda6 Formatting '/dev/sda6', fmt=qcow2 size=12884901888 backing_file='/var/lib/libvirt/images/f20.img' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 I then started the domain paused (as long as no guest I/O happens, the threshold cannot be exceeded, which gives me time to register a threshold and set up an event handler): # virsh start --paused f20 Domain f20 started # ./run tools/virsh -k0 domblkthreshold f20 vda 1 threshold of vda set to 1 bytes I was then able to play with domstats to see different thresholds as I registered them (such as playing with the parts-per-million or even with virsh 'percentage sugar'): # virsh -k0 domstats f20 --block | grep -C1 allo block.0.write-threshold=1 block.0.allocation=0 block.0.capacity=12884901888 In another window, I set up my event loop: $ ./run tools/virsh -k0 event f20 --loop --all then in the first window, I resumed things: # virsh resume f20 and got this message in the second window, after several seconds had elapsed (the first few seconds in the guest is under control of Grub, which doesn't write to disk; when it transitioned over to the Linux boot, disk activity started happening and easily causes the qcow2 wrapper to exceed the threshold I set earlier): $ ./run tools/virsh -k0 -c qemu:///system event --loop --all event 'lifecycle' for domain f20: Resumed Unpaused event 'lifecycle' for domain f20: Resumed Unpaused event 'write-threshold' for domain f20 disk vda (/dev/sda6): threshold 1 exceeded by 186608 bytes Sure enough, the registration had cleared, and the qcow2 wrapper had grown (it grew more than once in the process of booting, so by the time I re-queried, allocation had grown even further). # virsh -k0 domstats f20 --block | grep -C1 allo block.0.write-threshold=0 block.0.allocation=13434368 block.0.capacity=12884901888 I'd really like to see this series in 1.3.0 to commit to the API; there are still some further improvements we can make (in particular 'vda[1]' support, and also improving qemu event handling to re-grab a job lock if the cache of node names is not populated after a libvirtd restart), but as those improvements won't affect ABI, they can be deferred if they don't make it into 1.3.0. [I'm headed on vacation, so I will be really slow to respond in the next 3 weeks; someone else may need to push this if it is approved for 1.3.0] Eric Blake (10): threshold: new API virDomainBlockSetWriteThreshold threshold: expose new API in virsh threshold: new event object for tracking write threshold threshold: wire up threshold event in RPC threshold: add qemu capability bits threshold: learn a node name for a given qcow2 disk threshold: track an allocation node name for a storage source threshold: scrape threshold data from QMP threshold: add threshold event handling in qemu threshold: add write threshold setting in qemu daemon/remote.c | 51 +++ include/libvirt/libvirt-domain.h| 53 +++ src/conf/domain_event.c | 101 - src/conf/domain_event.h | 16 +- src/driver-hypervisor.h | 7 + src/libvirt-domain.c| 98 + src/libvirt_private.syms| 2 + src/libvirt_public.syms | 1 + src/qemu/qemu_capabilities.c| 12 +- src/qemu/qemu_capabilities.h| 4 +- src/qemu/qemu_domain.c | 105 ++ src/qemu/qemu_domain.h | 10 +- src/qemu/qemu_driver.c | 108 +- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 79 +++- src/qemu/qemu_monitor.h | 28 ++
[libvirt] [PATCH] vz: Fix error messages in libvirt log caused by unhadled events
We need to handle events with PIE_DISPATCHER type. Do not write error messages to log in case of unhandled event types. --- src/parallels/parallels_sdk.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 34605ea..d208c19 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1685,8 +1685,7 @@ prlsdkHandleVmEvent(parallelsConnPtr privconn, PRL_HANDLE prlEvent) return prlsdkHandleVmRemovedEvent(privconn, uuid); break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping event type %d, prlEventType); return PRL_ERR_FAILURE; } @@ -1721,6 +1720,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) switch (prlIssuerType) { case PIE_VIRTUAL_MACHINE: +case PIE_DISPATCHER: pret = prlsdkHandleVmEvent(privconn, prlEvent); break; default: -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vz: Fix error messages in libvirt log caused by unhadled events
22.06.2015 14:15, Mikhail Feoktistov пишет: We need to handle events with PIE_DISPATCHER type. Could you please shed some more light on why we need this. I guess it fixes some problem, so more explanation could be useful. Do not write error messages to log in case of unhandled event types. Again, please explain why we shouldn't log them as errors if we return them as failure. --- src/parallels/parallels_sdk.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 34605ea..d208c19 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1685,8 +1685,7 @@ prlsdkHandleVmEvent(parallelsConnPtr privconn, PRL_HANDLE prlEvent) return prlsdkHandleVmRemovedEvent(privconn, uuid); break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping event type %d, prlEventType); return PRL_ERR_FAILURE; } @@ -1721,6 +1720,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) switch (prlIssuerType) { case PIE_VIRTUAL_MACHINE: +case PIE_DISPATCHER: pret = prlsdkHandleVmEvent(privconn, prlEvent); break; default: -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] Storage fixes for libvirt.
From: Prerna Saxena pre...@linux.vnet.ibm.com Date: Mon, 22 Jun 2015 06:12:24 -0500 This is the second version of storage fixes. V1 : http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html Summary: Prerna Saxena (2): Storage: Introduce shadow vol for refresh while the main vol builds. Storage : Fix cloning of raw, sparse volumes src/storage/storage_backend.c | 10 +- src/storage/storage_driver.c | 20 +--- 2 files changed, 18 insertions(+), 12 deletions(-) Changelog: -- 1. Dropped patch 1/2 of v1 as per jtomko's suggestion; and introduced a new patch for shadow-volume based cloning. 2. Reworked patch 2/2 incorporating review comments. -- Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] virt-aa-helper: Fix permissions for vhost-user socket files
QEMU working in vhost-user mode communicates with the other end (i.e. some virtual router application) via unix domain sockets. This requires that permissions for the socket files are correctly written into /etc/apparmor.d/libvirt/libvirt-UUID.files. Signed-off-by: Michal Dubiel m...@semihalf.com --- Changes since v1: - Removed unnecessary stat() call and dead 'else' block src/security/virt-aa-helper.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..f39932e 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -32,7 +32,6 @@ #include unistd.h #include errno.h #include sys/types.h -#include sys/stat.h #include fcntl.h #include getopt.h #include sys/utsname.h @@ -542,7 +541,6 @@ array_starts_with(const char *str, const char * const *arr, const long size) static int valid_path(const char *path, const bool readonly) { -struct stat sb; int npaths, opaths; const char * const restricted[] = { /bin/, @@ -592,17 +590,6 @@ valid_path(const char *path, const bool readonly) if (!virFileExists(path)) { vah_warning(_(path does not exist, skipping file type checks)); -} else { -if (stat(path, sb) == -1) -return -1; - -switch (sb.st_mode S_IFMT) { -case S_IFSOCK: -return 1; -break; -default: -break; -} } opaths = sizeof(override)/sizeof(*(override)); @@ -1101,6 +1088,18 @@ get_files(vahControl * ctl) } } +for (i = 0; i ctl-def-nnets; i++) { +if (ctl-def-nets[i] +ctl-def-nets[i]-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER +ctl-def-nets[i]-data.vhostuser) { +virDomainChrSourceDefPtr vhu = ctl-def-nets[i]-data.vhostuser; + +if (vah_add_file_chardev(buf, vhu-data.nix.path, rw, + vhu-type) != 0) +goto cleanup; +} +} + if (ctl-def-virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i ctl-def-nnets; i++) { virDomainNetDefPtr net = ctl-def-nets[i]; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Storage : Fix cloning of raw, sparse volumes
From: Prerna Saxena pre...@linux.vnet.ibm.com Date: Mon, 22 Jun 2015 02:54:32 -0500 When virsh vol-clone is attempted on a raw file where capacity allocation, the resulting cloned volume has a size that matches the virtual-size of the parent; in place of matching its actual, disk size. This patch fixes the cloned disk to have same _allocated_size_ as the parent file from which it was cloned. Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739 Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com --- src/storage/storage_backend.c | 10 +- src/storage/storage_driver.c | 5 - 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ce59f63..c99b718 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -342,7 +342,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } -remain = vol-target.allocation; +remain = vol-target.capacity; if (inputvol) { int res = virStorageBackendCopyToFD(vol, inputvol, @@ -397,7 +397,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, bool reflink_copy) { -bool need_alloc = true; +bool need_alloc = !(inputvol (inputvol-target.capacity inputvol-target.allocation)); int ret = 0; unsigned long long remain; @@ -420,7 +420,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, * to writing zeroes block by block in case fallocate isn't * available, and since we're going to copy data from another * file it doesn't make sense to write the file twice. */ -if (vol-target.allocation) { +if (vol-target.allocation need_alloc) { if (fallocate(fd, 0, 0, vol-target.allocation) == 0) { need_alloc = false; } else if (errno != ENOSYS errno != EOPNOTSUPP) { @@ -433,14 +433,14 @@ createRawFile(int fd, virStorageVolDefPtr vol, } #endif -remain = vol-target.allocation; +remain = vol-target.capacity; if (inputvol) { /* allow zero blocks to be skipped if we've requested sparse * allocation (allocation capacity) or we have already * been able to allocate the required space. */ bool want_sparse = !need_alloc || -(vol-target.allocation inputvol-target.capacity); +(inputvol-target.allocation inputvol-target.capacity); ret = virStorageBackendCopyToFD(vol, inputvol, fd, remain, want_sparse, reflink_copy); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 4980546..c1dfe89 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1976,11 +1976,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (newvol-target.capacity origvol-target.capacity) newvol-target.capacity = origvol-target.capacity; -/* Make sure allocation is at least as large as the destination cap, - * to make absolutely sure we copy all possible contents */ -if (newvol-target.allocation origvol-target.capacity) -newvol-target.allocation = origvol-target.capacity; - if (!backend-buildVolFrom) { virReportError(VIR_ERR_NO_SUPPORT, %s, _(storage pool does not support -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Storage: Introduce shadow vol for refresh while the main vol builds.
From: Prerna Saxena pre...@linux.vnet.ibm.com Date: Thu, 18 Jun 2015 05:05:09 -0500 Libvirt periodically refreshes all volumes in a storage pool, including the volumes being cloned. While cloning a storage volume from parent, we drop pool locks. Subsequent volume refresh sometimes changes allocation for an ongoing copy, and leads to corrupt images. Fix: Introduce a shadow volume that isolates the volume object under refresh from the base which has a copy ongoing. Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com --- src/storage/storage_driver.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 57060ab..4980546 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1898,7 +1898,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, { virStoragePoolObjPtr pool, origpool = NULL; virStorageBackendPtr backend; -virStorageVolDefPtr origvol = NULL, newvol = NULL; +virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; virStorageVolPtr ret = NULL, volobj = NULL; unsigned long long allocation; int buildret; @@ -2010,6 +2010,17 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (backend-createVol(obj-conn, pool, newvol) 0) goto cleanup; +/* Make a shallow copy of the 'defined' volume definition, since the + * original allocation value will change as the user polls 'info', + * but we only need the initial requested values + */ +if (VIR_ALLOC(shadowvol) 0) { +newvol = NULL; +goto cleanup; +} + +memcpy(shadowvol, newvol, sizeof(*newvol)); + pool-volumes.objs[pool-volumes.count++] = newvol; volobj = virGetStorageVol(obj-conn, pool-def-name, newvol-name, newvol-key, NULL, NULL); @@ -2029,7 +2040,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, virStoragePoolObjUnlock(origpool); } -buildret = backend-buildVolFrom(obj-conn, pool, newvol, origvol, flags); +buildret = backend-buildVolFrom(obj-conn, pool, shadowvol, origvol, flags); storageDriverLock(); virStoragePoolObjLock(pool); -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [[libvirt-php][PATCH v2] qemu-agent-command] fixes for installation and add another libvirt function
On 17.06.2015 14:47, Vasiliy Tolstov wrote: * add libvirt_domain_qemu_agent_command * fix install target, because before this all stuff goes to /usr/usr dir These are semantically two separate changes and should go into separate patches. Can you please rebase, split and resend? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Jenkins build is back to normal : libvirt-syntax-check #3620
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/3620/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: caps: Fix syntax-check failure in version based capabilities
A single-line 'if' body should not be encased in curly braces. Our syntax-check enforces it. Introduced in 7f3515b4bb677d0ead1887547efc844 --- Pushed under the build-breaker rule. src/qemu/qemu_capabilities.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e4549ef..fa68d65 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1337,9 +1337,8 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); } -if (version = 2003000) { +if (version = 2003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI); -} return 0; } -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virQEMUCapsComputeCmdFlags: Indent correctly
There's a small formatting problem in the function. One line is not correctly indented. Fix this. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Initially, there were two chunks in this patch (and slightly different commit message), but Peter was faster than me a08e796bba85014446ad013c65eb58cd629c48a5. Pushed under trivial rule. src/qemu/qemu_capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index fa68d65..e7002a3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1206,7 +1206,7 @@ virQEMUCapsComputeCmdFlags(const char *help, if (version = 13000) { if (strstr(netdev, bridge)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE); - virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV); +virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV); } } -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: Fix permissions for vhost-user socket files
On 19 June 2015 at 21:30, Serge Hallyn serge.hal...@ubuntu.com wrote: Quoting Michal Dubiel (m...@semihalf.com): QEMU working in vhost-user mode communicates with the other end (i.e. some virtual router application) via unix domain sockets. This requires that permissions for the socket files are correctly written into /etc/apparmor.d/libvirt/libvirt-UUID.files. Signed-off-by: Michal Dubiel m...@semihalf.com --- src/security/virt-aa-helper.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..a097aa6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -592,19 +592,9 @@ valid_path(const char *path, const bool readonly) if (!virFileExists(path)) { vah_warning(_(path does not exist, skipping file type checks)); -} else { -if (stat(path, sb) == -1) +} else if (stat(path, sb) == -1) return -1; Hi, Why keep this bit? sb is not used later in the fn, and you already know that access(2) didn't return ENOENT. You are right, it is not needed. Thanks for pointing this out. I will update the patch accordingly. Regards, Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: Fix permissions for vhost-user socket files
Besides by changing from : } else { if ((x) == -1)) return -1; to } else if ((x) == -1) { return -1; Everything else inside the else if after the return -1 becomes dead Yes it becomes dead, but there is nothing left in the 'else if' block. Regards, Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [[libvirt-php][PATCH v1] fix memory functions] define memory functions
On 17.06.2015 11:58, Vasiliy Tolstov wrote: Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru --- src/libvirt-php.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index d46fbb9..093623c 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -136,6 +136,9 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_domain_create_xml, NULL) PHP_FE(libvirt_domain_memory_peek,NULL) PHP_FE(libvirt_domain_memory_stats,NULL) + PHP_FE(libvirt_domain_set_memory,NULL) + PHP_FE(libvirt_domain_set_max_memory,NULL) + PHP_FE(libvirt_domain_set_memory_flags,NULL) PHP_FE(libvirt_domain_block_stats,NULL) PHP_FE(libvirt_domain_block_resize,NULL) PHP_FE(libvirt_domain_block_job_abort,NULL) Extended the commit message, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] Introduce QEMU_CAPS_ARM_VIRT_PCI
On Sun, Jun 21, 2015 at 16:10:47 -0400, Cole Robinson wrote: On 06/11/2015 02:40 AM, Pavel Fedin wrote: This capability specifies that virt machine on ARM has PCI controller. Enabled when version is at least 2.3.0. Signed-off-by: Pavel Fedin p.fe...@samsung.com --- src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ca7a7c2..2eccc97 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -285,6 +285,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, dea-key-wrap, pci-serial, aarch64-off, + arm-virt-pci, ); @@ -1330,6 +1331,10 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); } +if (version = 2003000) { +virQEMUCapsSet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI); +} + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b5a7770..3c1a8b9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -229,6 +229,7 @@ typedef enum { QEMU_CAPS_DEA_KEY_WRAP = 187, /* -machine dea_key_wrap */ QEMU_CAPS_DEVICE_PCI_SERIAL = 188, /* -device pci-serial */ QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */ +QEMU_CAPS_ARM_VIRT_PCI = 190, /* ARM 'virt' machine has PCI bus */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; ACK and pushed, tweaked to avoid the conflict with .git (since additions to qemu_capabilities are always conflicting, it's better to get this in early) It breaks syntax-check since the 'if' has a single line body with braces around it. Also pushing a capabiltity without the code that will actually use it is not exactly a good idea since it might never be used if the next patch gets abandoned and since they are considered public we might be stuck with it forever. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: simplify json parsing
On 06/22/2015 11:10 AM, Eric Blake wrote: On 06/22/2015 11:01 AM, Eric Blake wrote: On 06/22/2015 09:05 AM, Michal Privoznik wrote: On 22.06.2015 15:52, Eric Blake wrote: On 06/20/2015 01:42 PM, Eric Blake wrote: Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with. Agreed. ACK to all the patches then. Thanks; pushed. And now seeing a test failure on 'jsontest' on a RHEL 6 box :( I'll investigate, since I just caused a build failure. Blech. Not only does RHEL 6 lack automatic trailing garbage detection added in yajl 2, it lacks yajl_get_bytes_consumed() added sometime after 1.0.7 and before 1.0.12. I've had to get creative on how to work around the issue, but I'll have patches ready for review shortly. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] json: reject javascript comments
We have been allowing javascript style comments in JSON ever since commit 9428f2c (v0.7.5), but qemu doesn't send them, and they are not strict JSON. Reject them for now; if we can later prove that it is worthwhile, we can reinstate it at that point (or even make it conditional, by adding a bool parameter to the libvirt entry point). * src/util/virjson.c (virJSONValueFromString): Don't enable comment parsing. * tests/jsontest.c (mymain): Test it. Signed-off-by: Eric Blake ebl...@redhat.com --- src/util/virjson.c | 4 ++-- tests/jsontest.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 4257b30..8d12fad 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1596,7 +1596,7 @@ virJSONValueFromString(const char *jsonstring) int rc; size_t len = strlen(jsonstring); # ifndef WITH_YAJL2 -yajl_parser_config cfg = { 1, 1 }; +yajl_parser_config cfg = { 0, 1 }; # endif VIR_DEBUG(string=%s, jsonstring); @@ -1604,7 +1604,7 @@ virJSONValueFromString(const char *jsonstring) # ifdef WITH_YAJL2 hand = yajl_alloc(parserCallbacks, NULL, parser); if (hand) { -yajl_config(hand, yajl_allow_comments, 1); +yajl_config(hand, yajl_allow_comments, 0); yajl_config(hand, yajl_dont_validate_strings, 0); } # else diff --git a/tests/jsontest.c b/tests/jsontest.c index 8ac0970..f6c2d84 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -418,6 +418,7 @@ mymain(void) DO_TEST_PARSE_FAIL(incomplete keyword, tr); DO_TEST_PARSE_FAIL(overdone keyword, [ truest ]); DO_TEST_PARSE_FAIL(unknown keyword, huh); +DO_TEST_PARSE_FAIL(comments, [ /* nope */\n1 // not this either\n]); DO_TEST_PARSE_FAIL(object with numeric keys, { 1:1, 2:1, 3:2 }); DO_TEST_PARSE_FAIL(unterminated object, { \1\:1, \2\:1, \3\:2); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/13] conf: new pci controller model pcie-root-port
This controller can be connected (at domain startup time only - not hotpluggable) only to a port on the pcie root complex (pcie-root in libvirt config), hence the new connect type VIR_PCI_CONNECT_TYPE_PCIE_ROOT. It provides a hotpluggable port that will accept any PCI or PCIe device. --- docs/formatdomain.html.in | 16 +-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_addr.c | 10 +-- src/conf/domain_addr.h | 5 +++- src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + .../qemuxml2argv-pcie-root-port.xml| 33 ++ tests/qemuxml2xmltest.c| 1 + 8 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fb3e2fb..e6c140a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2997,10 +2997,11 @@ p PCI controllers have an optional codemodel/code attribute with possible values codepci-root/code, codepcie-root/code, - codepci-bridge/code, or codedmi-to-pci-bridge/code. + codepcie-root-port/code, codepci-bridge/code, + or codedmi-to-pci-bridge/code. (pci-root and pci-bridge span class=sincesince 1.0.5/span, pcie-root and dmi-to-pci-bridge span class=sincesince - 1.1.2/span) + 1.1.2/span, pcie-root-port span class=sincesince 1.2.17/span) The root controllers (codepci-root/code and codepcie-root/code) have an optional codepcihole64/code element specifying how big (in kilobytes, or in the unit specified by codepcihole64/code's @@ -3054,6 +3055,17 @@ auto-determined by libvirt will be placed on this pci-bridge device. (span class=sincesince 1.1.2/span). /p +p + Domains with an implicit pcie-root can also add controllers + with codemodel='pcie-root-port'/code. This is a simple type of + bridge device that can connect only to one of the 31 slots on + the pcie-root bus on its upstream side, and makes a single + (PCIe, hotpluggable) port (at slot='0') available on the + downstream side. This controller can be used to provide a single + slot to later hotplug a PCIe device (but is not itself + hotpluggable - it must be in the configuration when the domain + is started). (span class=sincesince 1.2.17/span) +/p pre ... lt;devicesgt; diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f0f7400..6139dfb 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1751,6 +1751,7 @@ choice valuepci-bridge/value valuedmi-to-pci-bridge/value +valuepcie-root-port/value /choice /attribute /group diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 2be98c5..4b5e81e 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -183,9 +183,9 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: /* slots 1 - 31, no hotplug, PCIe only unless the address was * specified in user config *and* the particular device being - * attached also allows it + * attached also allows it. */ -bus-flags = VIR_PCI_CONNECT_TYPE_PCIE; +bus-flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_TYPE_PCIE_ROOT; bus-minSlot = 1; bus-maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; @@ -196,6 +196,12 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, bus-minSlot = 1; bus-maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: +/* provides one slot which is pcie and hotpluggable */ +bus-flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE; +bus-minSlot = 1; +bus-maxSlot = 1; +break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _(Invalid PCI controller model %d), model); diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index dcfd2e5..2a0ff96 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -39,6 +39,8 @@ typedef enum { /* PCI devices can connect to this bus */ VIR_PCI_CONNECT_TYPE_PCIE= 1 3, /* PCI Express devices can connect to this bus */ + VIR_PCI_CONNECT_TYPE_PCIE_ROOT = 1 4, + /* for devices that can only connect to pcie-root (i.e. root-port) */ } virDomainPCIConnectFlags; typedef struct { @@ -70,7 +72,8 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; * allowed, e.g. PCI, PCIe, switch */ # define
[libvirt] [PATCH 12/13] conf: new pcie-controller model pcie-switch-downstream-port
This controller can be connected only to a port on a pcie-switch-upstream-port. It provides a single hotpluggable port that will accept any PCI or PCIe device. --- docs/formatdomain.html.in | 31 +-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_addr.c | 6 src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + .../qemuxml2argv-pcie-switch-downstream-port.xml | 36 ++ tests/qemuxml2xmltest.c| 1 + 7 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 646c9ee..7e16dc4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2998,11 +2998,12 @@ PCI controllers have an optional codemodel/code attribute with possible values codepci-root/code, codepcie-root/code, codepcie-root-port/code, codepci-bridge/code, - codedmi-to-pci-bridge/code, or codepcie-switch-upstream-port/code. + codedmi-to-pci-bridge/code, codepcie-switch-upstream-port/code, or + codepcie-switch-downstream-port/code. (pci-root and pci-bridge span class=sincesince 1.0.5/span, pcie-root and dmi-to-pci-bridge span class=sincesince - 1.1.2/span, pcie-root-port and - pcie-switch-upstream-port span class=sincesince 1.2.17/span) + 1.1.2/span, pcie-root-port, pcie-switch-upstream-port, and + pcie-switch-downstream-port span class=sincesince 1.2.30/span) The root controllers (codepci-root/code and codepcie-root/code) have an optional codepcihole64/code element specifying how big (in kilobytes, or in the unit specified by codepcihole64/code's @@ -3058,14 +3059,24 @@ /p p Domains with an implicit pcie-root can also add controllers - with codemodel='pcie-root-port'/code. This is a simple type of - bridge device that can connect only to one of the 31 slots on - the pcie-root bus on its upstream side, and makes a single - (PCIe, hotpluggable) port (at slot='0') available on the - downstream side. This controller can be used to provide a single - slot to later hotplug a PCIe device (but is not itself + with codemodel='pcie-root-port'/code, + codemodel='pcie-switch-upstream-port'/code, + and codemodel='pcie-switch-downstream-port'/code. pcie-root-port + is a simple type of bridge device that can connect only to one + of the 31 slots on the pcie-root bus on its upstream side, and + makes a single (PCIe, hotpluggable) port (at slot='0') available + on the downstream side. This controller can be used to provide a + single slot to later hotplug a PCIe device (but is not itself hotpluggable - it must be in the configuration when the domain - is started). (span class=sincesince 1.2.17/span) + is started). pcie-switch-upstream-port is a more flexible (but + also more complex) device that can plug into any PCIe port on + the upstream side, and provides 31 ports on the downstream side + that accept only pcie-switch-downstream-port devices; each + pcie-switch-downstream-port device can only plug into a + pcie-switch-upstream-port on its upstream side, and on its + downstream side provides a single hotpluggable pcie port that + can accept any standard pci or pcie device. + (span class=sincesince 1.2.30/span) /p pre ... diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3f74cfc..490c0e7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1753,6 +1753,7 @@ valuedmi-to-pci-bridge/value valuepcie-root-port/value valuepcie-switch-upstream-port/value +valuepcie-switch-downstream-port/value /choice /attribute /group diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 59da745..b97a62c 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -211,6 +211,12 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, bus-minSlot = 1; bus-maxSlot = 31; break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: +/* provides one slot which is pcie and hotpluggable */ +bus-flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE; +bus-minSlot = 0; +bus-maxSlot = 0; +break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _(Invalid PCI controller model %d), model); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c0b8c92..8987e81 100644 --- a/src/conf/domain_conf.c +++
[libvirt] [PATCH 09/13] conf: new pci controller model pcie-switch-upstream-port
This controller can be connected to any PCIe port, but not to a standard PCI port, which is the reason for the new connect type VIR_PCI_CONNECT_TYPE_PCIE_ONLY. pcie-switch provides 31 ports (slot=1 to slot=31), which can only have pci controllers of model pcie-switch-downstream-port plugged into them, which is the reason for the other new connect type VIR_PCI_CONNECT_TYPE_PCIE_SWITCH. --- docs/formatdomain.html.in | 5 ++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_addr.c | 11 +++- src/conf/domain_addr.h | 6 +++- src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + .../qemuxml2argv-pcie-switch-upstream-port.xml | 32 ++ tests/qemuxml2xmltest.c| 1 + 8 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e6c140a..646c9ee 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2998,10 +2998,11 @@ PCI controllers have an optional codemodel/code attribute with possible values codepci-root/code, codepcie-root/code, codepcie-root-port/code, codepci-bridge/code, - or codedmi-to-pci-bridge/code. + codedmi-to-pci-bridge/code, or codepcie-switch-upstream-port/code. (pci-root and pci-bridge span class=sincesince 1.0.5/span, pcie-root and dmi-to-pci-bridge span class=sincesince - 1.1.2/span, pcie-root-port span class=sincesince 1.2.17/span) + 1.1.2/span, pcie-root-port and + pcie-switch-upstream-port span class=sincesince 1.2.17/span) The root controllers (codepci-root/code and codepcie-root/code) have an optional codepcihole64/code element specifying how big (in kilobytes, or in the unit specified by codepcihole64/code's diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6139dfb..3f74cfc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1752,6 +1752,7 @@ valuepci-bridge/value valuedmi-to-pci-bridge/value valuepcie-root-port/value +valuepcie-switch-upstream-port/value /choice /attribute /group diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 4b5e81e..59da745 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -57,6 +57,9 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, if (devFlags VIR_PCI_CONNECT_HOTPLUGGABLE) busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE; } +/* devices with PCIE_ONLY can't connect to PCI, even if fromConfig */ +if (devFlags VIR_PCI_CONNECT_TYPE_PCIE_ONLY) + devFlags |= VIR_PCI_CONNECT_TYPE_PCIE; /* If this bus doesn't allow the type of connection (PCI * vs. PCIe) required by the device, or if the device requires @@ -199,8 +202,14 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: /* provides one slot which is pcie and hotpluggable */ bus-flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE; +bus-minSlot = 0; +bus-maxSlot = 0; +break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: +/* 31 slots, can only accept pcie-switch-port, no hotplug */ +bus-flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH; bus-minSlot = 1; -bus-maxSlot = 1; +bus-maxSlot = 31; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 2a0ff96..8e9ca32 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -41,6 +41,10 @@ typedef enum { /* PCI Express devices can connect to this bus */ VIR_PCI_CONNECT_TYPE_PCIE_ROOT = 1 4, /* for devices that can only connect to pcie-root (i.e. root-port) */ + VIR_PCI_CONNECT_TYPE_PCIE_ONLY = 1 5, + /* devices that can't connect to pci even if manual config */ + VIR_PCI_CONNECT_TYPE_PCIE_SWITCH = 1 6, + /* devices that can only connect to a pcie-switch */ } virDomainPCIConnectFlags; typedef struct { @@ -73,7 +77,7 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; */ # define VIR_PCI_CONNECT_TYPES_MASK \ (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE | \ -VIR_PCI_CONNECT_TYPE_PCIE_ROOT) +VIR_PCI_CONNECT_TYPE_PCIE_ROOT | VIR_PCI_CONNECT_TYPE_PCIE_SWITCH) /* combination of all bits that could be used to connect a normal * endpoint device (i.e. excluding the connection possible between an diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0bdd51e..c0b8c92 100644 ---
[libvirt] [PATCH 11/13] qemu: add capabilities bit for device xio3130-downstream
The downstream ports of an x3130-upstream switch can each have one of these plugged into them (and that is the only place they can be connected). Each xio3130-downstream provides a single PCIe port that can have PCI or PCIe devices hotplugged into it. Apparently an entire set of x3130-upstream + several xio3130-downstreams can be hotplugged as a unit, but it's not clear to me yet how that would be done, since qemu only allows attaching a single device at a time. This device will be used to implement the pcie-switch-port model of pci controller. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 6 -- 10 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5a33efb..4103b6e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -289,6 +289,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, vhost-user-multiqueue, /* 190 */ ioh3420, x3130-upstream, + xio3130-downstream, ); @@ -1570,6 +1571,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { pci-serial, QEMU_CAPS_DEVICE_PCI_SERIAL }, { ioh3420, QEMU_CAPS_DEVICE_IOH3420 }, { x3130-upstream, QEMU_CAPS_DEVICE_X3130_UPSTREAM }, +{ xio3130-downstream, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3d28941..121e06b 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -232,6 +232,7 @@ typedef enum { QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */ QEMU_CAPS_DEVICE_IOH3420 = 191, /* -device ioh3420 */ QEMU_CAPS_DEVICE_X3130_UPSTREAM = 192, /* -device x3130-upstream */ +QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM = 193, /* -device xio3130-downstream */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index 78d7b82..ba16635 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -122,4 +122,5 @@ flag name='qxl-vga.vgamem_mb'/ flag name='ioh3420'/ flag name='x3130-upstream'/ +flag name='xio3130-downstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index 7cec7f9..51cd6d9 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -137,4 +137,5 @@ flag name='pci-serial'/ flag name='ioh3420'/ flag name='x3130-upstream'/ +flag name='xio3130-downstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index f5f0034..03d0a3e 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -138,4 +138,5 @@ flag name='pci-serial'/ flag name='ioh3420'/ flag name='x3130-upstream'/ +flag name='xio3130-downstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 9f0461a..e2f22e4 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -147,4 +147,5 @@ flag name='pci-serial'/ flag name='ioh3420'/ flag name='x3130-upstream'/ +flag name='xio3130-downstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 1b23b82..874a050 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -153,4 +153,5 @@ flag name='pci-serial'/ flag name='ioh3420'/ flag name='x3130-upstream'/ +flag name='xio3130-downstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index ff0427f..dd3bcda 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -153,4 +153,5 @@ flag name='pci-serial'/ flag name='ioh3420'/ flag name='x3130-upstream'/ +flag name='xio3130-downstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
[libvirt] [PATCH 08/13] qemu: add capabilities bit for device x3130-upstream
This is the upstream part of a PCIe switch. It connects to a PCIe port (but not PCI) on the upstream side, and can have up to 31 xio3130-downstream controllers (but no other types of devices) connected to its downstream side. This device will be used to implement the pcie-switch model of pci controller. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 6 -- 10 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a9a19e1..5a33efb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -288,6 +288,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, vhost-user-multiqueue, /* 190 */ ioh3420, + x3130-upstream, ); @@ -1568,6 +1569,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { pc-dimm, QEMU_CAPS_DEVICE_PC_DIMM }, { pci-serial, QEMU_CAPS_DEVICE_PCI_SERIAL }, { ioh3420, QEMU_CAPS_DEVICE_IOH3420 }, +{ x3130-upstream, QEMU_CAPS_DEVICE_X3130_UPSTREAM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d38505e..3d28941 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -231,6 +231,7 @@ typedef enum { QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */ QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */ QEMU_CAPS_DEVICE_IOH3420 = 191, /* -device ioh3420 */ +QEMU_CAPS_DEVICE_X3130_UPSTREAM = 192, /* -device x3130-upstream */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index a1fafa6..78d7b82 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -121,4 +121,5 @@ flag name='qxl.vgamem_mb'/ flag name='qxl-vga.vgamem_mb'/ flag name='ioh3420'/ +flag name='x3130-upstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index 824ef02..7cec7f9 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -136,4 +136,5 @@ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ flag name='ioh3420'/ +flag name='x3130-upstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index 7ef5199..f5f0034 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -137,4 +137,5 @@ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ flag name='ioh3420'/ +flag name='x3130-upstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 8c3d48e..9f0461a 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -146,4 +146,5 @@ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ flag name='ioh3420'/ +flag name='x3130-upstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 72f7625..1b23b82 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -152,4 +152,5 @@ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ flag name='ioh3420'/ +flag name='x3130-upstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index d81c80c..ff0427f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -152,4 +152,5 @@ flag name='qxl-vga.vgamem_mb'/ flag name='pci-serial'/ flag name='ioh3420'/ +flag name='x3130-upstream'/ /qemuCaps diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 1a39dce..56b27e5 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -168,4 +168,5 @@ flag name='pc-dimm'/ flag name='pci-serial'/ flag name='ioh3420'/ +flag name='x3130-upstream'/ /qemuCaps diff --git
[libvirt] [PATCH 02/13] qemu: always permit PCI devices to be manually assigned to a PCIe bus
When support for the pcie-root and dmi-to-pci-bridge buses on a Q35 machinetype was added, I was concerned that even though qemu at the time allowed plugging a PCI device into a PCIe port, that it might not be supported in the future. To prevent painful backtracking in the possible future where this happened, I disallowed such connections except in a few specific cases requested by qemu developers (indicated in the code with the flag VIR_PCI_CONNEcT_TYPE_EITHER_IF_CONFIG). Now that a couple years have passed, there is a clear message from qemu that there is no danger in allowing PCI devices to be plugged into PCIe ports. This patch eliminates VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG and changes the code to always allow PCI-PCIe or PCIe-PCI connection *when the PCI address is specified in the config. (For newly added devices that haven't yet been given a PCI address, the auto-placement still prefers using the correct type of bus). --- docs/formatdomain.html.in | 18 +++--- src/conf/domain_addr.c| 18 +++--- src/conf/domain_addr.h| 16 ++-- src/qemu/qemu_command.c | 6 ++ 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4e85b51..1dca2ac 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3034,17 +3034,21 @@ bus (for example, the machine types based on the Q35 chipset), the pcie-root controller with index=0 is auto-added to the domain's configuration. pcie-root has also no address, provides - 31 slots (numbered 1-31) and can only be used to attach PCIe - devices. In order to connect standard PCI devices on a system - which has a pcie-root controller, a pci controller + 31 slots (numbered 1-31) that can be used to attach PCIe or PCI + devices (although libvirt will never auto-assign a PCI device to + a PCIe slot, it will allow manual specification of such an + assignment). Devices connected to pcie-root cannot be + hotplugged. In order to make standard PCI slots available on a + system which has a pcie-root controller, a pci controller with codemodel='dmi-to-pci-bridge'/code is automatically - added. A dmi-to-pci-bridge controller plugs into a PCIe slot (as - provided by pcie-root), and itself provides 31 standard PCI - slots (which are not hot-pluggable). In order to have + added, usually at the defacto standard location of slot=0x1e. A + dmi-to-pci-bridge controller plugs into a PCIe slot (as provided + by pcie-root), and itself provides 31 standard PCI slots (which + also do not support device hotplug). In order to have hot-pluggable PCI slots in the guest system, a pci-bridge controller will also be automatically created and connected to one of the slots of the auto-created dmi-to-pci-bridge - controller; all guest devices with PCI addresses that are + controller; all guest PCI devices with addresses that are auto-determined by libvirt will be placed on this pci-bridge device. (span class=sincesince 1.1.2/span). /p diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index d657c7f..93c6043 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1,7 +1,7 @@ /* * domain_addr.c: helper APIs for managing domain device addresses * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -42,16 +42,21 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, { virErrorNumber errType = (fromConfig ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); -virDomainPCIConnectFlags flagsMatchMask = VIR_PCI_CONNECT_TYPES_MASK; -if (fromConfig) - flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG; +if (fromConfig) { +/* If the requested connection was manually specified in + * config, allow a PCI device to connect to a PCIe slot, or + * vice versa. + */ +if (busFlags VIR_PCI_CONNECT_TYPES_ENDPOINT) +busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT; +} /* If this bus doesn't allow the type of connection (PCI * vs. PCIe) required by the device, or if the device requires * hot-plug and this bus doesn't have it, return false. */ -if (!(devFlags busFlags flagsMatchMask)) { +if (!(devFlags busFlags VIR_PCI_CONNECT_TYPES_MASK)) { if (reportError) { if (devFlags VIR_PCI_CONNECT_TYPE_PCI) { virReportError(errType, @@ -174,8 +179,7 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, * specified in user config *and* the particular device being * attached also allows it */ -bus-flags = (VIR_PCI_CONNECT_TYPE_PCIE | -
[libvirt] [PATCH 10/13] qemu: support new pci controller model pcie-switch-upstream-port
this is backed by the qemu device x3130-upstream. --- src/qemu/qemu_command.c| 22 ++ .../qemuxml2argv-pcie-switch-upstream-port.args| 9 + tests/qemuxml2argvtest.c | 9 + 3 files changed, 40 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 01ac690..b4df65a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1604,6 +1604,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, */ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT; break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: +/* pcie-switch can only connect to a true + * pcie bus, and can't be hot-plugged. + */ +flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY; +break; default: break; } @@ -2355,6 +2361,12 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, /* pcie-root-port can only plug into pcie-root */ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT; break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: +/* pcie-switch really does need a real PCIe + * port, but it doesn't need to be pcie-root + */ +flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY; +break; default: flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; break; @@ -4625,6 +4637,16 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, virBufferAsprintf(buf, ioh3420,port=%d,chassis=%d,id=%s, def-idx, def-idx, def-info.alias); break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(The pcie-switch-upstream-port + (x3130-upstream) controller + is not supported in this QEMU binary)); +goto error; +} +virBufferAsprintf(buf, x3130-upstream,id=%s, def-info.alias); +break; } break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args new file mode 100644 index 000..d737b6b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \ +-device x3130-upstream,id=pci.3,bus=pcie.0,addr=0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ +-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \ +-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 999edff..b15352e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1469,6 +1469,15 @@ mymain(void) QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); +DO_TEST(pcie-switch-upstream-port, +QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, +QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, +QEMU_CAPS_DEVICE_IOH3420, +QEMU_CAPS_DEVICE_X3130_UPSTREAM, +QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, +QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, +QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); + DO_TEST(hostdev-scsi-lsi, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/13] qemu: support new pci controller model pcie-root-port
This is backed by the qemu device ioh3420. --- src/qemu/qemu_command.c | 20 .../qemuxml2argv-pcie-root-port.args | 10 ++ tests/qemuxml2argvtest.c | 7 +++ 3 files changed, 37 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4952797..01ac690 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1598,6 +1598,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, */ flags = VIR_PCI_CONNECT_TYPE_PCIE; break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: +/* pcie-root-port can only connect to pcie-root, isn't + * hot-pluggable + */ +flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT; +break; default: break; } @@ -2345,6 +2351,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, */ flags = VIR_PCI_CONNECT_TYPE_PCIE; break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: +/* pcie-root-port can only plug into pcie-root */ +flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT; +break; default: flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; break; @@ -4605,6 +4615,16 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } virBufferAsprintf(buf, i82801b11-bridge,id=%s, def-info.alias); break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(The pcie-root-port (ioh3420) controller + is not supported in this QEMU binary)); +goto error; +} +virBufferAsprintf(buf, ioh3420,port=%d,chassis=%d,id=%s, + def-idx, def-idx, def-info.alias); +break; } break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args new file mode 100644 index 000..30bec85 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \ +-device ioh3420,port=3,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=4,chassis=4,id=pci.4,bus=pcie.0,addr=0x3 \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ +-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \ +-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a90f9a6..999edff 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1461,6 +1461,13 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); +DO_TEST(pcie-root-port, +QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, +QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, +QEMU_CAPS_DEVICE_IOH3420, +QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, +QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, +QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); DO_TEST(hostdev-scsi-lsi, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/13] qemu: support new pci controller model pcie-switch-downstream-port
This is backed by the qemu device xio3130-downstream. It can only be connected to a pcie-switch-upstream-port (x3130-upstream) on the upstream side. --- src/qemu/qemu_command.c | 15 +++ .../qemuxml2argv-pcie-switch-downstream-port.args | 13 + tests/qemuxml2argvtest.c | 9 + 3 files changed, 37 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b4df65a..064adcb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2367,6 +2367,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, */ flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY; break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: +/* pcie-switch-port can only plug into pcie-switch */ +flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH; +break; default: flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; break; @@ -4647,6 +4651,17 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } virBufferAsprintf(buf, x3130-upstream,id=%s, def-info.alias); break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(The pcie-switch-downstream-port + (xio3130-downstream) controller + is not supported in this QEMU binary)); +goto error; +} +virBufferAsprintf(buf, xio3130-downstream,port=1,chassis=%d,id=%s, + def-idx, def-info.alias); +break; } break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args new file mode 100644 index 000..04b760c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args @@ -0,0 +1,13 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \ +-device x3130-upstream,id=pci.3,bus=pcie.0,addr=0x2 \ +-device xio3130-downstream,port=1,chassis=4,id=pci.4,bus=pci.3,addr=0x1 \ +-device xio3130-downstream,port=1,chassis=5,id=pci.5,bus=pci.3,addr=0x2 \ +-device xio3130-downstream,port=1,chassis=6,id=pci.6,bus=pci.3,addr=0x3 \ +-device xio3130-downstream,port=1,chassis=7,id=pci.7,bus=pci.3,addr=0x4 \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ +-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \ +-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b15352e..fc16152 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1477,6 +1477,15 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); +DO_TEST(pcie-switch-downstream-port, +QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, +QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, +QEMU_CAPS_DEVICE_IOH3420, +QEMU_CAPS_DEVICE_X3130_UPSTREAM, +QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM, +QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, +QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, +QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); DO_TEST(hostdev-scsi-lsi, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/13] qemu: refactor qemuBuildControllerDevStr to eliminate future duplicate code
The PCI case of the switch statement in this function contains another switch statement with a case for each model. Currently every model except pci-root and pcie-root have a check for index 0 (since only those two can have index==0), and the function should never be called for those two anyway. If we move the check for !pci[e]-root to the top of the pci case, then we can move the check for index 0 out of the individual model cases. This will save repeating that check for the three new controller models about to be added. --- src/qemu/qemu_command.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3886b4f..5e5cfe6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4581,13 +4581,20 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, break; case VIR_DOMAIN_CONTROLLER_TYPE_PCI: +if (def-model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || +def-model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(wrong function called for pci-root/pcie-root)); +return NULL; +} +if (def-idx == 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(index for pci controllers of model '%s' must be 0), + virDomainControllerModelPCITypeToString(def-model)); +goto error; +} switch (def-model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: -if (def-idx == 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(PCI bridge index should be 0)); -goto error; -} virBufferAsprintf(buf, pci-bridge,chassis_nr=%d,id=%s, def-idx, def-info.alias); break; @@ -4598,18 +4605,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, controller is not supported in this QEMU binary)); goto error; } -if (def-idx == 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(dmi-to-pci-bridge index should be 0)); -goto error; -} virBufferAsprintf(buf, i82801b11-bridge,id=%s, def-info.alias); break; -case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: -case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(wrong function called for pci-root/pcie-root)); -return NULL; } break; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv1.5 1/2] cpu:x86: fix specified features will disappear after migrate/restore
On 06/18/2015 09:29 PM, Jiri Denemark wrote: On Wed, Jun 17, 2015 at 22:22:44 +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1207095 We allow set the feature with the host-passthrough cpu, but won't save them in the migration xml, the features we specified will disappear after migrate/restore. This is because we skip the virCPUDefUpdateFeature if cpu mode is host-passthrough. Signed-off-by: Luyao Huang lhu...@redhat.com --- v1.5: just update the description. src/cpu/cpu_x86.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 2a14705..26601b6 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2087,8 +2087,7 @@ x86UpdateCustom(virCPUDefPtr guest, static int x86UpdateHostModel(virCPUDefPtr guest, - const virCPUDef *host, - bool passthrough) + const virCPUDef *host) { virCPUDefPtr oldguest = NULL; const struct x86_map *map; @@ -2124,7 +2123,7 @@ x86UpdateHostModel(virCPUDefPtr guest, } } } -for (i = 0; !passthrough i oldguest-nfeatures; i++) { +for (i = 0; i oldguest-nfeatures; i++) { if (virCPUDefUpdateFeature(guest, oldguest-features[i].name, oldguest-features[i].policy) 0) While this looks correct, save/restore (and likely migration too) with -cpu host has even more bugs (e.g., -cpu host turns into -cpu host,+... after save/restore) and I'd like to fix all that at once. Yes, there are more bugs here, seems i didn't notice that at that time. Thanks a lot for your review Jirka Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] qemu: add a check for slot and base when build dimm address
On 06/18/2015 08:12 PM, John Ferlan wrote: On 06/15/2015 08:33 AM, Luyao Huang wrote: When hot-plug a memory device, we don't check if there is a memory device have the same address with the memory device we want hot-pluged. Qemu forbid use/hot-plug 2 memory device with same slot or the same base(qemu side this elemnt named addr). Introduce a address check when build memory device qemu command line. Signed-off-by: Luyao Huang lhu...@redhat.com --- NOTE: Used the following for commit message: qemu: Add a check for slot and base dimm address conflicts When hotplugging a memory device, there wasn't a check to determine if there is a conflict with the address space being used by the to be added memory device and any existing device which is disallowed by qemu. This patch adds a check to ensure the new device address doesn't conflict with any existing device. Thanks for the message improvement v3: rename qemuBuildMemoryDeviceAddr to qemuCheckMemoryDimmConflict and remove the refactor. src/qemu/qemu_command.c | 37 + 1 file changed, 37 insertions(+) ACK Adjusted patch as shown below and pushed. Thanks a lot for your review and help John Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: fix a document issue as we support host-passthrough with features
On 06/18/2015 08:42 PM, Jiri Denemark wrote: On Wed, Jun 17, 2015 at 22:22:45 +0800, Luyao Huang wrote: From the documentation :With this mode, the CPU visible to the guest should be exactly the same as the host CPU even in the aspects that libvirt does not understand. So this place document need fix. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_process.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 64ee049..3cd0ff4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4024,8 +4024,10 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, bool ret = false; size_t i; -/* no features are passed to QEMU with -cpu host - * so it makes no sense to verify them */ +/* Although we allow set features with host-passthrough + * cpu mode, we allow user require/disable the feature + * that libvirt does not understand, so it makes no sense + * to verify them */ if (def-cpu def-cpu-mode == VIR_CPU_MODE_HOST_PASSTHROUGH) return true; NACK. We certainly don't want features to be just passed through without any validation if used with host-passthrough. We rather need to fix the code check the features used in a guest cpu element are all known to libvirt. Okay, get it Thanks a lot for your review. Jirka Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vz: Fix error messages in libvirt log caused by unhadled events
22.06.2015 14:22, Maxim Nestratov пишет: 22.06.2015 14:15, Mikhail Feoktistov пишет: We need to handle events with PIE_DISPATCHER type. Could you please shed some more light on why we need this. I guess it fixes some problem, so more explanation could be useful. If the configuration of the instance has been modified, for example added disk or network device, then hypervisor sends event with prlIssuerType = PIE_DISPATCHER and EventType = PET_DSP_EVT_VM_CONFIG_CHANGED We should handle this event in prlsdkHandleVmEvent function to update instance's XML config. Do not write error messages to log in case of unhandled event types. Again, please explain why we shouldn't log them as errors if we return them as failure. prlsdkHandleVmEvent is a common handler and it recieves many events. We handle only part of them. In case we don't handle event, we return error code to a caller function, which should make a decision whether to write error message to log. --- src/parallels/parallels_sdk.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 34605ea..d208c19 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1685,8 +1685,7 @@ prlsdkHandleVmEvent(parallelsConnPtr privconn, PRL_HANDLE prlEvent) return prlsdkHandleVmRemovedEvent(privconn, uuid); break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping event type %d, prlEventType); return PRL_ERR_FAILURE; } @@ -1721,6 +1720,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) switch (prlIssuerType) { case PIE_VIRTUAL_MACHINE: +case PIE_DISPATCHER: pret = prlsdkHandleVmEvent(privconn, prlEvent); break; default: -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] parallels: implementation of attach/detach network devices
On 06/17/2015 03:35 PM, Mikhail Feoktistov wrote: In this patch we add VIR_DOMAIN_DEVICE_NET handlers implementation for domainAttachDevice and domainDetachDevice callbacks. As soon as we don't support this operation for hypervisor type domains, we implement this functionality for containers only. In detach procedure we find network device by MAC address. Because PrlVmDevNet_GetMacAddress() returns MAC as a UTF-8 encoded null-terminated string, we use memcmp() to compare it. Also we remove corresponding virtual network by prlsdkDelNetAdapter call. --- src/vz/vz_driver.c | 16 +++ src/vz/vz_sdk.c| 127 src/vz/vz_sdk.h|4 ++ 3 files changed, 147 insertions(+), 0 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index cef3c77..d9ddd4f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1117,6 +1117,14 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkAttachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network attach failed)); We always report errors where they occurred, so I think you skip this message, it will be reported inside prlsdkAttachNet. +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be attached), @@ -1186,6 +1194,14 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkDetachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network detach failed)); +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be detached), diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 090a3ad..0f92e52 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2914,6 +2914,133 @@ static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) PrlHandle_Free(vnet); } +int prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net) +{ +int ret = -1; +parallelsDomObjPtr privdom = dom-privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +if (!IS_CT(dom-def)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(network device cannot be attached)); +goto cleanup; +} + +job = PrlVm_BeginEdit(privdom-sdkdom); +if (PRL_FAILED(waitJob(job))) +goto cleanup; + +ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def)); +if (ret == 0) { +job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); +if (PRL_FAILED(waitJob(job))) { +ret = -1; +goto cleanup; +} +} + + cleanup: There is no cleanup here, so you can just return from the function instead of jumping to cleanup section. +return ret; +} + +static int +prlsdkGetNetIndex(PRL_HANDLE sdkdom, virDomainNetDefPtr net) +{ +int idx = -1; +PRL_RESULT pret; +PRL_UINT32 netCount; +PRL_UINT32 i; +PRL_HANDLE adapter = PRL_INVALID_HANDLE; +PRL_UINT32 len; +char adapterMac[PRL_MAC_STRING_BUFNAME]; +char netMac[PRL_MAC_STRING_BUFNAME]; Using names 'adapterMac' and 'netMac' is a little confusing, because net and adapter are synonyms in this context, I think something like 'expectedMac' would be better name for netMac. + +prlsdkFormatMac(net-mac, netMac); +pret = PrlVmCfg_GetNetAdaptersCount(sdkdom, netCount); +prlsdkCheckRetGoto(pret, cleanup); + +for (i = 0; i netCount; ++i) { + +pret = PrlVmCfg_GetNetAdapter(sdkdom, i, adapter); +prlsdkCheckRetGoto(pret, cleanup); + +len = sizeof(adapterMac); +memset(adapterMac, 0, sizeof(adapterMac)); +pret = PrlVmDevNet_GetMacAddress(adapter, adapterMac, len); +prlsdkCheckRetGoto(pret, cleanup); + +if (memcmp(adapterMac, netMac, PRL_MAC_STRING_BUFNAME)) { + +PrlHandle_Free(adapter); +adapter = PRL_INVALID_HANDLE; +continue; +} + +idx = i; +break; +} + + cleanup: +PrlHandle_Free(adapter); +return idx; +} + +static int prlsdkDelNetAdapter(PRL_HANDLE sdkdom, int idx) +{ +int ret = -1; +PRL_RESULT pret; +PRL_HANDLE sdknet = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetNetAdapter(sdkdom, idx, sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_Remove(sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +ret = 0; + + cleanup: +
[libvirt] Add hook for network update event
Hey guys, This is duplicate from Bugzilla (https://bugzilla.redhat.com/show_bug.cgi?id=1181539), but I would like to ask you if my patch looks good. virsh net-update starts iptables rules reload for NAT network system. This event doesn't start network hooks, so all previous custom iptables rules become unavailable. How reproducible: Add /etc/libvirt/hooks/network with: --- #!/bin/bash echo `date` $0 $@ /var/log/libvirt.log --- Restart libvirt and run virsh net-update command: virsh net-update default modify ip-dhcp-host --live --config host mac='52:54:00:97:eb:95' name='test' ip='192.168.122.253'/ Actual results: /var/log/libvirt.log log file doesn't contain events from virsh net-update command. Expected results: /var/log/libvirt.log log file should contain event. Additional info: This bug brakes iptables hooks hack for FORWARD chain with NAT network. Patch file is attached. It was tested with libvirt 1.2.15 --- bridge_driver.c 2015-05-27 03:26:03.0 +0200 +++ libvirt-1.2.16/src/network/bridge_driver.c 2015-06-22 09:18:28.829897135 +0200 @@ -3301,6 +3301,10 @@ if (needFirewallRefresh networkAddFirewallRules(network-def) 0) goto cleanup; +if (needFirewallRefresh networkRunHook(network, NULL, NULL, VIR_HOOK_NETWORK_OP_STARTED, VIR_HOOK_SUBOP_BEGIN) 0) +goto cleanup; + + if (flags VIR_NETWORK_UPDATE_AFFECT_CONFIG) { /* save updated persistent config to disk */ if (virNetworkSaveConfig(driver-networkConfigDir, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Explicitly format the isa-fdc controller for newer q35 machines
Since QEMU commit ea96bc6 [1]: i386: drop FDC in pc-q35-2.4+ if neither it nor floppy drives are wanted the floppy controller is no longer implicit. Specify it explicitly on the command line if the machine type version is 2.4 or later. Note that libvirt's floppy drives do not result in QEMU implying the controller, because libvirt uses if=none instead of if=floppy. [1] http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ea96bc6 --- src/qemu/qemu_command.c| 31 + src/qemu/qemu_domain.c | 22 src/qemu/qemu_domain.h | 1 + .../qemuxml2argv-boot-floppy-q35.args | 12 +++ .../qemuxml2argv-boot-floppy-q35.xml | 40 ++ .../qemuxml2argv-bootindex-floppy-q35.args | 11 ++ .../qemuxml2argv-bootindex-floppy-q35.xml | 40 ++ tests/qemuxml2argvtest.c | 9 + 8 files changed, 159 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootindex-floppy-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootindex-floppy-q35.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 418682d..77568e0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8809,9 +8809,9 @@ qemuBuildCommandLine(virConnectPtr conn, * List of controller types that we add commandline args for, * *in the order we want to add them*. * - * We don't add an explicit FD controller because the - * provided PIIX4 device already includes one. It isn't possible to - * remove the PIIX4. + * The floppy controller is implicit on PIIX4 and older Q35 + * machines. For newer Q35 machines it is added out of the + * controllers loop, after the floppy drives. * * We don't add PCI/PCIe root controller either, because it's * implicit, but we do add PCI bridges and other PCI @@ -8832,6 +8832,8 @@ qemuBuildCommandLine(virConnectPtr conn, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virBuffer boot_buf = VIR_BUFFER_INITIALIZER; char *boot_order_str = NULL, *boot_opts_str = NULL; +virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; +char *fdc_opts_str = NULL; VIR_DEBUG(conn=%p driver=%p def=%p mon=%p json=%d qemuCaps=%p migrateFrom=%s migrateFD=%d @@ -9781,8 +9783,12 @@ qemuBuildCommandLine(virConnectPtr conn, disk-info.alias) 0) goto error; -virCommandAddArg(cmd, -global); -virCommandAddArgFormat(cmd, isa-fdc.%s, optstr); +if (!qemuDomainMachineNeedsFDC(def)) { +virCommandAddArg(cmd, -global); +virCommandAddArgFormat(cmd, isa-fdc.%s, optstr); +} else { +virBufferAsprintf(fdc_opts, %s,, optstr); +} VIR_FREE(optstr); if (bootindex) { @@ -9792,8 +9798,12 @@ qemuBuildCommandLine(virConnectPtr conn, bootindex) 0) goto error; -virCommandAddArg(cmd, -global); -virCommandAddArgFormat(cmd, isa-fdc.%s, optstr); +if (!qemuDomainMachineNeedsFDC(def)) { +virCommandAddArg(cmd, -global); +virCommandAddArgFormat(cmd, isa-fdc.%s, optstr); +} else { +virBufferAsprintf(fdc_opts, %s,, optstr); +} VIR_FREE(optstr); } } else { @@ -9807,6 +9817,13 @@ qemuBuildCommandLine(virConnectPtr conn, } } } +/* Newer Q35 machine types require an explicit FDC controller */ +virBufferTrim(fdc_opts, ,, -1); +if ((fdc_opts_str = virBufferContentAndReset(fdc_opts))) { +virCommandAddArg(cmd, -device); +virCommandAddArgFormat(cmd, isa-fdc,%s, fdc_opts_str); +VIR_FREE(fdc_opts_str); +} } else { for (i = 0; i def-ndisks; i++) { char dev[NAME_MAX]; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6213fd9..50468af 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3230,6 +3230,28 @@ qemuDomainMachineIsI440FX(const virDomainDef *def) } +bool +qemuDomainMachineNeedsFDC(const virDomainDef *def) +{ +char *p = STRSKIP(def-os.machine, pc-q35-); + +if (p) { +if (STRPREFIX(p, 1.) || +STRPREFIX(p,
Re: [libvirt] Add hook for network update event
On 22.06.2015 12:23, kay wrote: Hey guys, This is duplicate from Bugzilla (https://bugzilla.redhat.com/show_bug.cgi?id=1181539), but I would like to ask you if my patch looks good. virsh net-update starts iptables rules reload for NAT network system. This event doesn't start network hooks, so all previous custom iptables rules become unavailable. How reproducible: Add /etc/libvirt/hooks/network with: --- #!/bin/bash echo `date` $0 $@ /var/log/libvirt.log --- Restart libvirt and run virsh net-update command: virsh net-update default modify ip-dhcp-host --live --config host mac='52:54:00:97:eb:95' name='test' ip='192.168.122.253'/ Actual results: /var/log/libvirt.log log file doesn't contain events from virsh net-update command. Expected results: /var/log/libvirt.log log file should contain event. Additional info: This bug brakes iptables hooks hack for FORWARD chain with NAT network. Patch file is attached. It was tested with libvirt 1.2.15 libvirt_bridge_hook.patch --- bridge_driver.c 2015-05-27 03:26:03.0 +0200 +++ libvirt-1.2.16/src/network/bridge_driver.c2015-06-22 09:18:28.829897135 +0200 @@ -3301,6 +3301,10 @@ if (needFirewallRefresh networkAddFirewallRules(network-def) 0) goto cleanup; +if (needFirewallRefresh networkRunHook(network, NULL, NULL, VIR_HOOK_NETWORK_OP_STARTED, VIR_HOOK_SUBOP_BEGIN) 0) +goto cleanup; + This is not quite right. The network is not starting. You need to introduce new operation type to virHookNetworkOpType, e.g. VIR_HOOK_NETWORK_OP_UPDATED. Also, we don't like long lines ('make all syntax-check check' is your friend). Otherwise looking good. Also, do you mind proposing this as a regular patch to the list? + if (flags VIR_NETWORK_UPDATE_AFFECT_CONFIG) { /* save updated persistent config to disk */ if (virNetworkSaveConfig(driver-networkConfigDir, Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vz: Fix error messages in libvirt log caused by unhadled events
On 22.06.2015 16:48, Maxim Nestratov wrote: 22.06.2015 16:44, Nikolay Shirokovskiy пишет: On 22.06.2015 16:00, Mikhail Feoktistov wrote: 22.06.2015 14:22, Maxim Nestratov пишет: 22.06.2015 14:15, Mikhail Feoktistov пишет: We need to handle events with PIE_DISPATCHER type. Could you please shed some more light on why we need this. I guess it fixes some problem, so more explanation could be useful. If the configuration of the instance has been modified, for example added disk or network device, then hypervisor sends event with prlIssuerType = PIE_DISPATCHER and EventType = PET_DSP_EVT_VM_CONFIG_CHANGED We should handle this event in prlsdkHandleVmEvent function to update instance's XML config. This was breaked by my commit c71f5f8cee3fe4078c3ab8aa7c46de7210b468c6 The only event of issuer type PIE_VIRTUAL_MACHINE is performace event, all others events issuer type is PIE_VIRTUAL_MACHINE. I was miguided by event names: PET_DSP_EVT_VM_PERFSTATS PET_DSP_EVT_VM_UNREGISTERED So what are others? PIE_DISPATCHER Do not write error messages to log in case of unhandled event types. Again, please explain why we shouldn't log them as errors if we return them as failure. prlsdkHandleVmEvent is a common handler and it recieves many events. We handle only part of them. In case we don't handle event, we return error code to a caller function, which should make a decision whether to write error message to log. This also restores behaviour before named commit. --- src/parallels/parallels_sdk.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 34605ea..d208c19 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1685,8 +1685,7 @@ prlsdkHandleVmEvent(parallelsConnPtr privconn, PRL_HANDLE prlEvent) return prlsdkHandleVmRemovedEvent(privconn, uuid); break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping event type %d, prlEventType); return PRL_ERR_FAILURE; } @@ -1721,6 +1720,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) switch (prlIssuerType) { case PIE_VIRTUAL_MACHINE: +case PIE_DISPATCHER: pret = prlsdkHandleVmEvent(privconn, prlEvent); break; default: -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vz: Fix error messages in libvirt log caused by unhadled events
On 22.06.2015 16:00, Mikhail Feoktistov wrote: 22.06.2015 14:22, Maxim Nestratov пишет: 22.06.2015 14:15, Mikhail Feoktistov пишет: We need to handle events with PIE_DISPATCHER type. Could you please shed some more light on why we need this. I guess it fixes some problem, so more explanation could be useful. If the configuration of the instance has been modified, for example added disk or network device, then hypervisor sends event with prlIssuerType = PIE_DISPATCHER and EventType = PET_DSP_EVT_VM_CONFIG_CHANGED We should handle this event in prlsdkHandleVmEvent function to update instance's XML config. This was breaked by my commit c71f5f8cee3fe4078c3ab8aa7c46de7210b468c6 The only event of issuer type PIE_VIRTUAL_MACHINE is performace event, all others events issuer type is PIE_VIRTUAL_MACHINE. I was miguided by event names: PET_DSP_EVT_VM_PERFSTATS PET_DSP_EVT_VM_UNREGISTERED Do not write error messages to log in case of unhandled event types. Again, please explain why we shouldn't log them as errors if we return them as failure. prlsdkHandleVmEvent is a common handler and it recieves many events. We handle only part of them. In case we don't handle event, we return error code to a caller function, which should make a decision whether to write error message to log. This also restores behaviour before named commit. --- src/parallels/parallels_sdk.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 34605ea..d208c19 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1685,8 +1685,7 @@ prlsdkHandleVmEvent(parallelsConnPtr privconn, PRL_HANDLE prlEvent) return prlsdkHandleVmRemovedEvent(privconn, uuid); break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping event type %d, prlEventType); return PRL_ERR_FAILURE; } @@ -1721,6 +1720,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) switch (prlIssuerType) { case PIE_VIRTUAL_MACHINE: +case PIE_DISPATCHER: pret = prlsdkHandleVmEvent(privconn, prlEvent); break; default: -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vz: Fix error messages in libvirt log caused by unhadled events
22.06.2015 16:44, Nikolay Shirokovskiy пишет: On 22.06.2015 16:00, Mikhail Feoktistov wrote: 22.06.2015 14:22, Maxim Nestratov пишет: 22.06.2015 14:15, Mikhail Feoktistov пишет: We need to handle events with PIE_DISPATCHER type. Could you please shed some more light on why we need this. I guess it fixes some problem, so more explanation could be useful. If the configuration of the instance has been modified, for example added disk or network device, then hypervisor sends event with prlIssuerType = PIE_DISPATCHER and EventType = PET_DSP_EVT_VM_CONFIG_CHANGED We should handle this event in prlsdkHandleVmEvent function to update instance's XML config. This was breaked by my commit c71f5f8cee3fe4078c3ab8aa7c46de7210b468c6 The only event of issuer type PIE_VIRTUAL_MACHINE is performace event, all others events issuer type is PIE_VIRTUAL_MACHINE. I was miguided by event names: PET_DSP_EVT_VM_PERFSTATS PET_DSP_EVT_VM_UNREGISTERED So what are others? Do not write error messages to log in case of unhandled event types. Again, please explain why we shouldn't log them as errors if we return them as failure. prlsdkHandleVmEvent is a common handler and it recieves many events. We handle only part of them. In case we don't handle event, we return error code to a caller function, which should make a decision whether to write error message to log. This also restores behaviour before named commit. --- src/parallels/parallels_sdk.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 34605ea..d208c19 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1685,8 +1685,7 @@ prlsdkHandleVmEvent(parallelsConnPtr privconn, PRL_HANDLE prlEvent) return prlsdkHandleVmRemovedEvent(privconn, uuid); break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping event type %d, prlEventType); return PRL_ERR_FAILURE; } @@ -1721,6 +1720,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) switch (prlIssuerType) { case PIE_VIRTUAL_MACHINE: +case PIE_DISPATCHER: pret = prlsdkHandleVmEvent(privconn, prlEvent); break; default: -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: simplify json parsing
On 06/20/2015 01:42 PM, Eric Blake wrote: Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor_json.c | 164 --- 1 file changed, 61 insertions(+), 103 deletions(-) @@ -271,7 +269,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon, memset(msg, 0, sizeof(msg)); -exe = virJSONValueObjectGet(cmd, execute); +exe = virJSONValueObjectGetObject(cmd, execute); if (exe) { if (!(id = qemuMonitorNextCommandID(mon))) goto cleanup; This hunk is spurious (the 'execute' member is a string, not an object); I've removed it locally from my tree. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Handle isa-fdc removal from new q35 machine types
https://bugzilla.redhat.com/show_bug.cgi?id=1227880 Ján Tomko (2): Separate isa-fdc options generation Explicitly format the isa-fdc controller for newer q35 machines src/qemu/qemu_command.c| 50 -- src/qemu/qemu_domain.c | 22 ++ src/qemu/qemu_domain.h | 1 + .../qemuxml2argv-boot-floppy-q35.args | 12 ++ .../qemuxml2argv-boot-floppy-q35.xml | 40 + .../qemuxml2argv-bootindex-floppy-q35.args | 11 + .../qemuxml2argv-bootindex-floppy-q35.xml | 40 + tests/qemuxml2argvtest.c | 9 8 files changed, 172 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootindex-floppy-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootindex-floppy-q35.xml -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Separate isa-fdc options generation
For the implicit controller, we set them via -global. Separating them will allow reuse for explicit fdc controller as well. No functional impact apart from one extra allocation. --- src/qemu/qemu_command.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5444638..418682d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9776,18 +9776,25 @@ qemuBuildCommandLine(virConnectPtr conn, if (withDeviceArg) { if (disk-bus == VIR_DOMAIN_DISK_BUS_FDC) { +if (virAsprintf(optstr, drive%c=drive-%s, +disk-info.addr.drive.unit ? 'B' : 'A', +disk-info.alias) 0) +goto error; + virCommandAddArg(cmd, -global); -virCommandAddArgFormat(cmd, isa-fdc.drive%c=drive-%s, - disk-info.addr.drive.unit - ? 'B' : 'A', - disk-info.alias); +virCommandAddArgFormat(cmd, isa-fdc.%s, optstr); +VIR_FREE(optstr); if (bootindex) { +if (virAsprintf(optstr, bootindex%c=%d, +disk-info.addr.drive.unit +? 'B' : 'A', +bootindex) 0) +goto error; + virCommandAddArg(cmd, -global); -virCommandAddArgFormat(cmd, isa-fdc.bootindex%c=%d, - disk-info.addr.drive.unit - ? 'B' : 'A', - bootindex); +virCommandAddArgFormat(cmd, isa-fdc.%s, optstr); +VIR_FREE(optstr); } } else { virCommandAddArg(cmd, -device); -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [[PATCH v6] autocreate tap device for VIR_DOMAIN_NET_TYPE_ETHERNET] autocreate tap device for VIR_DOMAIN_NET_TYPE_ETHERNET
On 19.06.2015 14:00, Michal Privoznik wrote: On 19.06.2015 10:21, Vasiliy Tolstov wrote: If a user specify ehernet device create it via libvirt and run script if it provided. After this commit user does not need to run external script to create tap device or add root to qemu process. Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru --- src/qemu/qemu_command.c | 142 +++- src/qemu/qemu_hotplug.c | 13 ++--- src/qemu/qemu_process.c | 6 ++ 3 files changed, 101 insertions(+), 60 deletions(-) Just a couple of my findings: - this is not rebased onto the HEAD from the date you've sent this - 'make check' is broken with this patch applied. I'm tracing it down, to see how many functions do we need to mock. Interesting, so I've tried to mock some functions, but some are not that easy. I mean, for instance, if I wanted to mock qemuNetworkIfaceConnect() - I couldn't. The control jumps into the original function defined in qemu_command.c. I'm not sure how to resolve this. Does anybody have a bright idea? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] lxc: setsid() usage
Hi! Why is libvirt-lxc issuing a setsid() in lxcContainerSetupFDs()? To me it seems like a hack to have a controlling TTY if PID 1 is /bin/bash. If one runs a sysv init style distro (like Debian) in libvirt-lxc the setsid() has a major downside, when getty spawns a login shell on /dev/tty1 it cannot become the controlling tty. Hence, if one presses ctrl-c in such a session, the container will reboot. Interestingly it does not happen when a systemd distro is used. Maybe because systemd completely closes and reopens the TTY? Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list