Re: [libvirt] [PATCH v6 07/13] conf: Introduce parser, formatter for uid and fid
在 2018/10/11 下午7:45, Andrea Bolognani 写道:
On Fri, 2018-09-28 at 16:46 +0800, Yi Min Zhao wrote:
This patch introduces new XML parser/formatter functions. Uid is
16-bit and non-zero. Fid is 32-bit. They are the two attributes of zpci
which is introduced as PCI address element. Zpci element is parsed and
formatted along with PCI address. And add the related test cases.
Signed-off-by: Yi Min Zhao
---
No internal reviews this time around? :)
Yes, I just want to quickly know if this change is exact.
[...]
@@ -227,9 +288,22 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
goto cleanup;
}
+
if (!virPCIDeviceAddressIsEmpty(addr) &&
!virPCIDeviceAddressIsValid(addr, true))
goto cleanup;
Spurious whitespace change.
Oh...good catch.
[...]
@@ -6528,7 +6528,19 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
break;
}
-virBufferAddLit(buf, "/>\n");
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci)) {
+virBufferAddLit(buf, ">\n");
+virBufferAdjustIndent(buf, 2);
+virBufferAsprintf(buf,
+ "\n",
+ info->addr.pci.zpci.uid,
+ info->addr.pci.zpci.fid);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "/>\n");
+}
This looks like a perfect candidate for virXMLFormatElement(). You
should probably convert the original code to use that function in a
separate commit, then start actually using a child buffer here.
Sure.
[...]
@@ -2566,6 +2566,7 @@ virPCIHeaderTypeToString;
virPCIIsVirtualFunction;
virPCIStubDriverTypeFromString;
virPCIStubDriverTypeToString;
+virZPCIDeviceAddressIsEmpty;
You can move virZPCIDeviceAddressIsValid() to util/virpci and
export it too, to be consistent with virPCIDeviceAddress*().
It has been moved to util/virpci. Isn't it?
[...]
@@ -272,4 +275,6 @@ VIR_DEFINE_AUTOPTR_FUNC(virPCIDevice, virPCIDeviceFree)
VIR_DEFINE_AUTOPTR_FUNC(virPCIDeviceAddress, virPCIDeviceAddressFree)
VIR_DEFINE_AUTOPTR_FUNC(virPCIEDeviceInfo, virPCIEDeviceInfoFree)
+bool virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr);
+
Please place this further up in the file, eg. after
virPCIDeviceAddressParse().
Sure.
Everything else looks good.
--
Yi Min
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 09/13] conf: Allocate/release 'uid' and 'fid' in PCI address
在 2018/10/11 下午10:50, Andrea Bolognani 写道: On Fri, 2018-09-28 at 16:46 +0800, Yi Min Zhao wrote: # conf/device_conf.h +virDeviceInfoPCIAddressExtensionIsPresent; +virDeviceInfoPCIAddressExtensionIsWanted; virDeviceInfoPCIAddressIsPresent; virDeviceInfoPCIAddressIsWanted; virDomainDeviceAddressIsValid; @@ -119,6 +121,9 @@ virDomainCCWAddressSetFree; virDomainPCIAddressBusIsFullyReserved; virDomainPCIAddressBusSetModel; virDomainPCIAddressEnsureAddr; +virDomainPCIAddressExtensionReleaseAddr; +virDomainPCIAddressExtensionReserveAddr; +virDomainPCIAddressExtensionReserveNextAddr; I'm not quite quire we need to export these functions. With the recent changes, we've gotten to the point that we're not even passing a virZPCIDeviceAddress to them, but rather they have the very same signature as the regular virPCIDeviceAddress... So it should be possible to just make them static and only call them from the virDomainPCIAddressReserveAddr() and friends, right? Which is where I was hoping we could eventually get. Or did I miss something? I think this would make things complex. If either PCI address or zPCI address exists, we have to do more checks for calling virDomainPCIAddressReserveAddr(). And there are amounts of code calling ***IsWanted() to call ***ReserveNext***(). I think keeping them separately is better. Everything else looks good. -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC v2] qemu: fix deadlock when waiting in non async jobs
On 10/8/18 4:10 AM, Nikolay Shirokovskiy wrote: > Block job abort operation can not handle properly qemu crashes when waiting > for > abort/pivot completion. Deadlock scenario is next: > > - qemuDomainBlockJobAbort waits for pivot/abort completion > - qemu crashes, then qemuProcessBeginStopJob broadcasts for VM condition and > then waits for job condition (taken by qemuDomainBlockJobAbort) > - qemuDomainBlockJobAbort awakes but nothing really changed, VM is still > active (vm->def->id != -1) so thread starts waiting for completion again. > Now two threads are in deadlock. > > First let's remove broadcast in qemuProcessBeginStopJob. It is simply wrong > because it is not set any condition before broadcast so that awaked threads > can > not detect any changes. Crashing domain during async job will continue to be > handled properly because destroy job can run concurrently with async job and > destroy job calls qemuProcessStop which sets vm->def->id to -1 and broadcasts. Hmm... Although blockjobs are not my area of expertise, I do seem to have a knack for reading and commenting on patches with these edge conditions. At first, taken alone this made it seem like separate patches are required, but maybe not depending on the relationship described above. As an aside, for this paragraph hunk you could call out commit 4d0c535a3 where this is/was introduced. Beyond the refactor, the broadcast was added; however, it seems it was done so on purpose since the broadcast would seemingly allowing something to be awoken. Beyond that - take away the scenario you describing where QEMU crashes. In the normal path, if you remove the broadcast, then do things work properly? Since a block job would set @priv->blockjob when a block job starts and is cleared during qemuBlockJobEventProcess processing when status is VIR_DOMAIN_BLOCK_JOB_COMPLETED, VIR_DOMAIN_BLOCK_JOB_FAILED, or VIR_DOMAIN_BLOCK_JOB_CANCELED. What about setting a @priv->blockjobAbort when the abort starts. Then perhaps processMonitorEOFEvent or qemuProcessHandleMonitorEOF can handle that properly so that we don't deadlock. Perhaps or hopefully, Jirka or Peter will comment too with this bump. John > > Second let's introduce flag that EOF is received and broadcast after that. > Now non async jobs can check this flag in wait loop. > > Signed-off-by: Nikolay Shirokovskiy > > --- > > Diff from v1: > > - patches 1 and 2 are already merged > - don't bother with reporting monitor EOF reason to user as most of > time it is simply "unexpected eof" (this implies dropping patch 3) > - drop patch 5 as we now always report "domain is being stopped" > in qemuDomainObjWait > - don't signal on monitor error for simplicity (otherwise we need to report > something more elaborate that "domain is being stopped" as we don't > kill domain on monitor errors. On the other hand I guess monitor > error is rare case to handle it right now) > - keep virDomainObjWait for async jobs > > It's a bit uneven that for async jobs domain is destroyed concurrently and for > non async jobs it will be actually destroyed after job get completed. Also if > non async job needs issuing commands to qemu on cleanup then we will send > these > commands in vain polluting logs etc because qemu process in not running at > this > moment but typical check (virDomainObjIsActive) will think it is still > running. > > Domain is destroyed (qemuProcessStop) in a job due to patches [1] and [2]. > However AFAIU it is not neccessary. If qemuProcessStop does not drop VM lock > then we don't need extra job to make qemuProcessStop and main job not > interleave. And we can drop the lock now only in qemuDomainObjBeginNestedJob > in > qemuProcessStop which is introduced in [2]. AFAIU we can fix issues mentioned > in > [2] the other way for example like it is done for qemu agent - we save agent > monitor reference on stack for entering/exiting agent monitor. > > So I wonder can we instead of this fix remove job for qemuProcessStop and run > destroying domain cuncurrently for non async jobs too. > > [1] > commit 8c9ff9960b29d4703a99efdd1cadcf6f48799cc0 > Author: Jiri Denemark > Date: Thu Feb 11 15:32:48 2016 +0100 > > qemu: Process monitor EOF in a job > > [2] > commit 81f50cb92d16643bcd749e3ab5b404b8b7cec643 > Author: Jiri Denemark > Date: Thu Feb 11 11:20:28 2016 +0100 > > qemu: Avoid calling qemuProcessStop without a job > > src/qemu/qemu_domain.c | 39 +++ > src/qemu/qemu_domain.h | 4 > src/qemu/qemu_driver.c | 2 +- > src/qemu/qemu_hotplug.c | 4 ++-- > src/qemu/qemu_process.c | 9 + > 5 files changed, 51 insertions(+), 7 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 939b2a3..aead72b 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -13534,3 +13534,42 @@ > qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason) > > return
Re: [libvirt] [RFC PATCH auto partition NUMA guest domains v1 0/2] auto partition guests providing the host NUMA topology
On Tue, 25 Sep 2018 14:37:15 +0200 Jiri Denemark wrote: > On Tue, Sep 25, 2018 at 12:02:40 +0200, Wim Ten Have wrote: > > From: Wim ten Have > > > > This patch extends the guest domain administration adding support > > to automatically advertise the host NUMA node capabilities obtained > > architecture under a guest by creating a vNUMA copy. > > I'm pretty sure someone would find this useful and such configuration is > perfectly valid in libvirt. But I don't think there is a compelling > reason to add some magic into the domain XML which would automatically > expand to such configuration. It's basically a NUMA placement policy and > libvirt generally tries to avoid including any kind of policies and > rather just provide all the mechanisms and knobs which can be used by > applications to implement any policy they like. > > > The mechanism is enabled by setting the check='numa' attribute under > > the CPU 'host-passthrough' topology: > > > > Anyway, this is definitely not the right place for such option. The > 'check' attribute is described as > > "Since 3.2.0, an optional check attribute can be used to request a > specific way of checking whether the virtual CPU matches the > specification." > > and the new 'numa' value does not fit in there in any way. > > Moreover the code does the automatic NUMA placement at the moment > libvirt parses the domain XML, which is not the right place since it > would break migration, snapshots, and save/restore features. Howdy, thanks for your fast response. I was Out Of Office for a while unable to reply earlier. The beef of this code does indeed not belong under the domain code and should rather move into the NUMA specific code where check='numa' is simply badly chosen. Also whilst OOO it occurred to me that besides auto partitioning the host into a vNUMA replica there's probably even other configuration target we may introduce reserving a single NUMA-node out of the nodes reserved for a guest to configure. So my plan is to come back asap with reworked code. > We have existing placement attributes for vcpu and numatune/memory > elements which would have been much better place for implementing such > feature. And event cpu/numa element could have been enhanced to support > similar configuration. Going over libvirt documentation I am more appealed with vcpu area. As said let me rework and return with better approach/RFC asap. Rgds, - Wim10H. > Jirka > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ANNOUNCE: virt-manager 2.0.0 released
I'm happy to announce the release of virt-manager 2.0.0! virt-manager is a desktop application for managing KVM, Xen, and LXC virtualization via libvirt. The release can be downloaded from: http://virt-manager.org/download/ The 2.0.0 isn't hugely significant here, the app will largely look the same to most people. Internally we had the big change of python3 support which definitely bumps up the minimum supported host version virt-manager can run on. I also took the opportunity to remove some uncommonly used features from virt-manager's UI, chief among them the host interface management UI. In practice I don't think this will really impact anyone because it didn't work that well to begin with. More details are in this thread: https://www.redhat.com/archives/virt-tools-list/2018-October/msg00032.html The big changes in this release include: - Finish port to Python 3 (Radostin Stoyanov, Cole Robinson) - Improved VM defaults for supported OS: q35 PCIe, usb3, CPU host-model - Search based OS selection UI for new VMs (Daniel P. Berrangé, Cole Robinson) - Track OS name for lifetime of domain in XML - Host interface management UI has been completely removed - Show domain IP on interface details page (Lin Ma, Cole Robinson) - More efficient stats polling with AllDomainStats (Simon Kobyda, Cole Robinson) - TPM device model and backend UI (Marc-André Lureau, Stefan Berger) - Show connection state in UI (Lin Ma) - Show attached devices in UI (Lin Ma) - UI option to plug/unplug VM nic link (Simon Kobyda) - UI support for disk discard and detect_zeroes (Povilas Kanapickas, Lin Ma) - Improved SUSE --location URL/ISO detection (Charles Arnold) - cli and UI support for SCSI persistent reservations (Lin Ma) - cli: Add --network mtu.size= option (Anya Harter) - cli: Add --disk driver.copy_on_read (Anya Harter) - cli: Add --disk geometry support (Anya Harter) - cli: Add --sound codec support (Anya Harter) - cli: Add --hostdev net/char/block for LXC (Lubomir Rintel) - cli: Add --memorybacking access_mode and source_type (Marc-André Lureau) - cli: Add --boot rebootTimout (Yossi Ovadia) - cli: Add --destroy-on-exit Thanks to everyone who has contributed to this release through testing, bug reporting, submitting patches, and otherwise sending in feedback! Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 13/19] conf: Add resctrl monitor configuration
On 10/15/18 11:25 AM, Wang, Huaqiang wrote: > > On 10/13/2018 6:29 AM, John Ferlan wrote: >> >> On 10/12/18 3:10 AM, Wang, Huaqiang wrote: -Original Message- [...] IOW: What is cache_occupancy measuring? Each cache? The entire thing? If there's no cache elements, then what? >>> cache_occupancy is measuring based on cache bank. For Intel 2 socket >>> xeon CPU, >>> it is considered as two cache banks, one cache bank per socket. The >>> typical >>> output for each monitor of this case is: >>> >>> cpu.cache.0.name=vcpus_1 >>> cpu.cache.0.vcpus=1 >>> cpu.cache.0.bank.count=2 <--- 2 cache banks >>> cpu.cache.0.bank.0.id=0 <--- bank.id.0 cache_occypancy >>> cpu.cache.0.bank.0.bytes=9371648 _| >>> cpu.cache.0.bank.1.id=1 <--- bank.id.1 cache_occypancy >>> cpu.cache.0.bank.1.bytes=1081344 _| >>> >>> If you want to know the total cache occupancy for VM vcpu threads of >>> this >>> monitor, you need to add them up. >>> >> So if you have: >> >> >> >> what do you get in output for cache_occupancy? 0 + 1? > > Yes. Output is sum of two vcpus. > > for cache bank 0 > vcpus_0-1.bank.0.bytes = vcpus_0.bank.0.bytes + vcpus_1.bank.0.bytes > for cache bank 1 > vcpus_0-1.bank.1.bytes = vcpus_0.bank.1.bytes + vcpus_1.bank.1.bytes > >> I honestly think this just needs to be simplified as much as possible. > > "I honestly think this just needs to be simplified as much as possible." > > I reconsidered your comment ( in above line), do you mean the XML > configuration for 'monitor' need to be simplified also? > This is/was a comment regarding default stuff which you are removing. > What I think is, even after the removal of 'default monitor' and > 'default allocation' concepts, the XML > configuration for monitors (with type 'all', 'm-to-n', 'one to one') > still need such kind of arrangement. > > Take an example, a VM has 4 vcpus, vcpu 0 and 1 run cache sensitive > workload, and wants to hold > private L3 caches, and there is no specific requirement for left vcpus > but still need a monitoring on > the cache usage. > > Then we could create an cache allocation for vcpu 0 and 1 as well as a > monitor on getting the > actual cache that these two vcpus used. For vcpu 2 and 3, create a > monitor for it. > > The XML configurations are: (no change in general rules comparing to my > previous examples) > > > > > > > > > > > Any suggestion from you is welcome. > I'm not sure what the question is and I'm not sure it matters at this point. If you only create an allocation for any or entry, then that's all that'll be reported which is what I was trying to point out. Its not that something else may or may not exist, it's what gets reported and can be queried via the XML. > When you monitor specific vcpus within a cachetune, then you get what? >>> In this case, the monitor you created only monitors the specific vcpus >>> you added for monitor. >>> >>> Following two configurations satisfy your scenario, and the only monitor >>> will detect the cache usage of thread of vcpu 2. >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >> Perhaps my question was mistyped or misinterpreted. In the above top >> example, if we have , then do the values in >> have any impact on the calculation as opposed to if they weren't >> there? > > I perhaps still not understand you well ... > There will have significant influence for the output of monitor if > entry > exist and if vcpu2-4 demands much more caches that allocation can offer; > If the > cache that the allocation offers is much bigger than vcpu2-4 actually > used, the > influence will be tiny. > > But in another case, that, if there is no 'cache' entries, just showing > in the second > example, it still influenced by the cache that the 'allocation' offers. > Its difference > with the first example is: the top example is using the cache resources > allocated > by the allocation of itself, while the second example uses the > allocation of resources > defined in /sys/fs/resctrl/schemata, and this cache is shared by > multiple system tasks. > The question was related to how is defined and trying to further describe my feeling that default was necessary. >> >>> If the cachetune has no specific cache entries, you get what? >>> If no cache entry in cachetune, it will also get vcpu threads' cache >>> utilization information based on cache bank. >>> No cache entry specified for the cachetune, means it will use the cache >>> allocating policy of default cache allocation, which is file >>> /sys/fs/resctrl/schemata. >>> >>> If valid cache entries are provided in cachetune, then an allocation >>> will >>> be created for the threads of vcpu listed in 'vcpus' >>> attribute. Supposing the allocation is the directory /sys/fs/resctrl/p0, >>> then the
Re: [libvirt] [PATCHv5 13/19] conf: Add resctrl monitor configuration
On 10/13/2018 6:29 AM, John Ferlan wrote: On 10/12/18 3:10 AM, Wang, Huaqiang wrote: -Original Message- From: John Ferlan [mailto:jfer...@redhat.com] Sent: Thursday, October 11, 2018 4:58 AM To: Wang, Huaqiang ; libvir-list@redhat.com Cc: Feng, Shaohe ; Niu, Bing ; Ding, Jian-feng ; Zang, Rui Subject: Re: [libvirt] [PATCHv5 13/19] conf: Add resctrl monitor configuration On 10/9/18 6:30 AM, Wang Huaqiang wrote: Introducing element under to represent a cache monitor. Supports two kind of monitors, which are, monitor under default allocation or monitor under particular allocation. I still don't see what the big difference is with singling out default. Does it really matter - what advantage is there. Having a monitor for 'n' vcpus is a choice. Monitor supervises the cache or memory bandwidth usage for At this point memory bandwidth is not in play... Yes. Will remove the memory BW related words. interested vcpu thread set, if the vcpu thread set is belong to some resctrl allocation, then the monitor will be created under this allocation, that is, creating a resctrl monitoring group directory under the directory of '@alloc->path/mon_group'. Otherwise, the monitor will be created under default allocation. This is becoming increasing difficult to describe/decipher - makes me wonder who would really use it. Monitor in default allocation could be commonly used when we want to observe the resource usage for VM vcpu threads that are not specified with any dedicated resource limitation through CAT or MBA technology. And another common case is it will be used if CAT/MBA is not supported but CMT/MBM is enabled in libvirt. The reason I have introduced the monitor in previous patch and further introducing the monitor in default allocation in this patch is the monitor n two different allocations has different behavior. Let's focus on CAT and CMT technology in my below lines, since MBA and MBM are very similar cases to them. As we know, before the CAT technology was introduced, any process in Linux is sharing the L3 cache along with all other active processes. After CAT is enabled in libvirt, it has the capability to apply cache isolation and assign dedicated amount of cache to some key VM vcpu threads. If we want to observe the real L3 cache usage information, then we need the help of monitor. == Monitor usage case 1: monitor in default allocation == If you want to get the cache utilization data before applying any cache isolation to the VM vcpu threads, you need to create a monitor in the default allocation, because you haven't create any cache allocation. == Monitor usage case 2: monitor in non-default allocation == If you have created a cache allocation for specific VM vcpu treads and not sure how many cache-lines these VM vcpu threads are really used, you need to create a monitor under the this allocation to get real cache usage information. If you find your VM vcpu threads only used a small part of cache that have allocated, you might think about to reduce its allocation. == Usage for default monitor and non-default monitor == Since we have introduced the 'default monitor' and 'non-default monitor' concepts in previous patches, and now, you can monitor the cache usage for all VM vcpu threads that added to this allocation, and also you has the capability to monitor a subset of vcpu list of this allocation. Without 'default monitor', it is impossible to get the cache usage for all vcpu threads in the allocation and at the same time get the cache usage for some highly interested vcpu threads of allocation. "Default monitor" is in name only. It's just "a" monitor for all the threads in cachetune (or memorytune eventually); whereas, a "non default monitor" is "a" monitor for specific vcpus within the range in cachetune. Agree. As stated in reply of patch10, 'default monitor' and will be removed, and related code will be cleaned. Thus your description is that a monitor can be a monitor all vcpus or a monitor for some subset of of all vcpus. A describes which vcpus of a cachetune (or memorytune) can be monitored - there are 3 options - "all", "m-to-n", or "one-to-one". Thanks for suggestion. Then no things will be mentioned as 'default', code and document will be cleaned accordingly. What they relate to in the filesystem is the magic of the code of paths to the data. What data structures are called or how this is described in docs just doesn't seem to need to make a delineation. Agree. == Monitor usage case 3: allocating dedicated cache and monitoring its usage of one VM, and getting its influence over another VM== Think about the scenario that there are two VMs, it is a known information that one VM is very cache sensitive and don't want to share cache with other workloads, and for another VM, we have no knowledge about cache requirement, but it is required to monitor the cache usage for the two VMs. With the concepts introduced until now. We need to
Re: [libvirt] [PATCH 00/11] Allow modification of IOThread polling values (redux)
On 10/15/2018 04:28 PM, John Ferlan wrote: > > ping? > > Any takers or thoughts? No review, but I think it makes a lot of sense to expose these tuneables. > > Tks, > > John > > > On 10/7/18 9:00 AM, John Ferlan wrote: >> This series attempts to resurrect the concept of being able to modify >> the IOThread polling parameters; however, in a slightly different >> manner than the previous series posted by posted by Pavel Hrdina >> : >> >> https://www.redhat.com/archives/libvir-list/2017-February/msg01047.html >> >> The work is prompted by continued pleas found in the bz: >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1545732 >> >> to provide some way to modify the paremters without needing to supply >> QEMU command line pass through values. >> >> It's accepted that the values being changed are fairly or extremely >> low level type knobs; however, it's also shown that by being able to >> turn the knob it is possible for certain, specific appliances to be >> able to gain a performance benefit for the thread at the expense of >> other competing threads. >> >> Unlike the previous series, this series does not attempt to save the >> polling values in the guest XML. Rather, it only modifies the live >> guest's IOThread with new values. It also doesn't provide the polling >> values in a virsh iothread* command, rather it uses the domstats >> in order to fetch and display the values. The theory being this >> leaves the onus on the higher level appliance/application to provide >> the "proper guidance" and "concerns" related to changing the values >> to the consumer. Not saving the values means whatever values that >> are chosen do not "live" in perpetuity. Once the guest is shut down >> or the IOThread removed from guest, the hypervisor default values >> take over again. Perhaps not a perfect situation in terms of what >> the bz requests; however, storage of default values that could >> cause performance issues is not an optimal situation. So this I >> figured is a "comprimise" of sorts. >> >> If it's still felt that no we don't want to do this, then fine, >> but please in doing so own the bz, state your case, and close it. >> I'm 50/50 on it, but figured at least I'd present this option and >> see what the concensus was. >> >> John Ferlan (11): >> qemu: Check for and return IOThread polling values if available >> qemu: Split qemuDomainGetIOThreadsLive >> qemu: Implement the ability to return IOThread stats >> virsh: Add ability to display IOThread stats >> lib: Introduce virDomainSetIOThreadParams >> qemu: Add monitor functions to set IOThread params >> qemu: Alter qemuDomainChgIOThread to take enum instead of bool >> qemu: Alter qemuDomainChgIOThread to take qemuMonitorIOThreadInfo >> qemu: Detect whether iothread polling is supported >> qemu: Introduce qemuDomainSetIOThreadParams >> tools: Add virsh iothreadset command >> >> include/libvirt/libvirt-domain.h | 45 ++ >> src/driver-hypervisor.h | 8 + >> src/libvirt-domain.c | 109 + >> src/libvirt_public.syms | 5 + >> src/qemu/qemu_capabilities.c | 2 + >> src/qemu/qemu_capabilities.h | 1 + >> src/qemu/qemu_driver.c| 393 -- >> src/qemu/qemu_monitor.c | 19 + >> src/qemu/qemu_monitor.h | 6 + >> src/qemu/qemu_monitor_json.c | 48 +++ >> src/qemu/qemu_monitor_json.h | 4 + >> src/remote/remote_driver.c| 1 + >> src/remote/remote_protocol.x | 21 +- >> src/remote_protocol-structs | 10 + >> .../caps_2.10.0.aarch64.xml | 1 + >> .../caps_2.10.0.ppc64.xml | 1 + >> .../caps_2.10.0.s390x.xml | 1 + >> .../caps_2.10.0.x86_64.xml| 1 + >> .../caps_2.11.0.s390x.xml | 1 + >> .../caps_2.11.0.x86_64.xml| 1 + >> .../caps_2.12.0.aarch64.xml | 1 + >> .../caps_2.12.0.ppc64.xml | 1 + >> .../caps_2.12.0.s390x.xml | 1 + >> .../caps_2.12.0.x86_64.xml| 1 + >> .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + >> .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + >> .../caps_2.9.0.x86_64.xml | 1 + >> .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + >> .../caps_3.0.0.riscv32.xml| 1 + >> .../caps_3.0.0.riscv64.xml| 1 + >> .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + >> .../caps_3.0.0.x86_64.xml | 1 + >> tools/virsh-domain-monitor.c | 7 + >> tools/virsh-domain.c | 110 + >> tools/virsh.pod | 47 ++- >> 35 files changed, 810
Re: [libvirt] [PATCH] conf: Fix bug in finding alloc through matching vcpus
On 10/12/18 6:24 AM, Wang Huaqiang wrote: > The @alloc object returned by virDomainResctrlVcpuMatch is not > properly referenced and un-referenced in virDomainCachetuneDefParse. > > This patch fixes this problem. > > Signed-off-by: Wang Huaqiang > --- > src/conf/domain_conf.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > Reviewed-by: John Ferlan (and pushed) John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/11] Allow modification of IOThread polling values (redux)
ping? Any takers or thoughts? Tks, John On 10/7/18 9:00 AM, John Ferlan wrote: > This series attempts to resurrect the concept of being able to modify > the IOThread polling parameters; however, in a slightly different > manner than the previous series posted by posted by Pavel Hrdina > : > > https://www.redhat.com/archives/libvir-list/2017-February/msg01047.html > > The work is prompted by continued pleas found in the bz: > > https://bugzilla.redhat.com/show_bug.cgi?id=1545732 > > to provide some way to modify the paremters without needing to supply > QEMU command line pass through values. > > It's accepted that the values being changed are fairly or extremely > low level type knobs; however, it's also shown that by being able to > turn the knob it is possible for certain, specific appliances to be > able to gain a performance benefit for the thread at the expense of > other competing threads. > > Unlike the previous series, this series does not attempt to save the > polling values in the guest XML. Rather, it only modifies the live > guest's IOThread with new values. It also doesn't provide the polling > values in a virsh iothread* command, rather it uses the domstats > in order to fetch and display the values. The theory being this > leaves the onus on the higher level appliance/application to provide > the "proper guidance" and "concerns" related to changing the values > to the consumer. Not saving the values means whatever values that > are chosen do not "live" in perpetuity. Once the guest is shut down > or the IOThread removed from guest, the hypervisor default values > take over again. Perhaps not a perfect situation in terms of what > the bz requests; however, storage of default values that could > cause performance issues is not an optimal situation. So this I > figured is a "comprimise" of sorts. > > If it's still felt that no we don't want to do this, then fine, > but please in doing so own the bz, state your case, and close it. > I'm 50/50 on it, but figured at least I'd present this option and > see what the concensus was. > > John Ferlan (11): > qemu: Check for and return IOThread polling values if available > qemu: Split qemuDomainGetIOThreadsLive > qemu: Implement the ability to return IOThread stats > virsh: Add ability to display IOThread stats > lib: Introduce virDomainSetIOThreadParams > qemu: Add monitor functions to set IOThread params > qemu: Alter qemuDomainChgIOThread to take enum instead of bool > qemu: Alter qemuDomainChgIOThread to take qemuMonitorIOThreadInfo > qemu: Detect whether iothread polling is supported > qemu: Introduce qemuDomainSetIOThreadParams > tools: Add virsh iothreadset command > > include/libvirt/libvirt-domain.h | 45 ++ > src/driver-hypervisor.h | 8 + > src/libvirt-domain.c | 109 + > src/libvirt_public.syms | 5 + > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_driver.c| 393 -- > src/qemu/qemu_monitor.c | 19 + > src/qemu/qemu_monitor.h | 6 + > src/qemu/qemu_monitor_json.c | 48 +++ > src/qemu/qemu_monitor_json.h | 4 + > src/remote/remote_driver.c| 1 + > src/remote/remote_protocol.x | 21 +- > src/remote_protocol-structs | 10 + > .../caps_2.10.0.aarch64.xml | 1 + > .../caps_2.10.0.ppc64.xml | 1 + > .../caps_2.10.0.s390x.xml | 1 + > .../caps_2.10.0.x86_64.xml| 1 + > .../caps_2.11.0.s390x.xml | 1 + > .../caps_2.11.0.x86_64.xml| 1 + > .../caps_2.12.0.aarch64.xml | 1 + > .../caps_2.12.0.ppc64.xml | 1 + > .../caps_2.12.0.s390x.xml | 1 + > .../caps_2.12.0.x86_64.xml| 1 + > .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + > .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + > .../caps_2.9.0.x86_64.xml | 1 + > .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + > .../caps_3.0.0.riscv32.xml| 1 + > .../caps_3.0.0.riscv64.xml| 1 + > .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + > .../caps_3.0.0.x86_64.xml | 1 + > tools/virsh-domain-monitor.c | 7 + > tools/virsh-domain.c | 110 + > tools/virsh.pod | 47 ++- > 35 files changed, 810 insertions(+), 44 deletions(-) > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] docs: Enhance polkit documentation to describe secondary connection
https://bugzilla.redhat.com/show_bug.cgi?id=1631608
Since commit 8259255 usage of a primary connection driver for
a virConnect has been modified to open (virConnectOpen) and use
a connection to the specific driver in order to handle the API
calls to/for that driver. This causes some confusion and issues
for ACL polkit rule scripts to know exactly which driver by
name will be used.
Add some documentation describing the processing of the primary
and secondary connection as well as the list of the connect_driver
names used for each driver.
Signed-off-by: John Ferlan
---
docs/aclpolkit.html.in | 117 +
docs/libvirt.css | 1 +
2 files changed, 118 insertions(+)
diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in
index ee00b98461..ac54f125da 100644
--- a/docs/aclpolkit.html.in
+++ b/docs/aclpolkit.html.in
@@ -287,6 +287,123 @@
+Hypervisor Driver connect_driver
+
+ The connect_driver parameter describes the
+ client's remote Connection Driver
+ name based on the URI used for the
+ connection.
+
+
+ Since 4.1.0, when calling an API
+ outside the scope of the primary connection driver, the
+ primary driver will attempt to open a secondary connection
+ to the specific API driver in order to process the API. For
+ example, when hypervisor domain processing needs to make an
+ API call within the storage driver or the network filter driver
+ an attempt to open a connection to the "storage" or "nwfilter"
+ driver will be made. Similarly, a "storage" primary connection
+ may need to create a connection to the "secret" driver in order
+ to process secrets for the API. If successful, then calls to
+ those API's will occur in the connect_driver context
+ of the secondary connection driver rather than in the context of
+ the primary driver. This affects the connect_driver
+ returned from rule generation from the action.loookup
+ function. The following table provides a list of the various
+ connection drivers and the connect_driver name
+ used by each regardless of primary or secondary connection.
+ The access denied error message from libvirt will list the
+ connection driver by name that denied the access.
+
+
+Connection Driver Name
+
+
+
+ Connection Driver
+ connect_driver name
+
+
+
+
+ bhyve
+ bhyve
+
+
+ esx
+ ESX
+
+
+ hyperv
+ Hyper-V
+
+
+ interface
+ interface
+
+
+ libxl
+ xenlight
+
+
+ lxc
+ LXC
+
+
+ network
+ network
+
+
+ nodedev
+ nodedev
+
+
+ nwfilter
+ NWFilter
+
+
+ openvz
+ OPENVZ
+
+
+ phyp
+ PHYP
+
+
+ qemu
+ QEMU
+
+
+ secret
+ secret
+
+
+ storage
+ storage
+
+
+ uml
+ UML
+
+
+ vbox
+ VBOX
+
+
+ vmware
+ VMWARE
+
+
+ vz
+ vz
+
+
+ xenapi
+ XenAPI
+
+
+
+
User identity attributes
diff --git a/docs/libvirt.css b/docs/libvirt.css
index b2ed33926a..e590b33cfb 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -393,6 +393,7 @@ table.acl {
table.acl tr, table.acl td {
padding: 0.3em;
+border: 1px solid #ccc;
}
table.acl thead {
--
2.17.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] access: Modify the VIR_ERR_ACCESS_DENIED to include driverName
https://bugzilla.redhat.com/show_bug.cgi?id=1631606
Changes made to manage and utilize a secondary connection
driver to APIs outside the scope of the primary connection
driver have resulted in some confusion processing polkit rules
since the simple "access denied" error message doesn't provide
enough of a clue when combined with the "authentication failed:
access denied by policy" as to which connection driver refused
or failed the ACL check.
In order to provide some context, let's modify the existing
"access denied" error returne from the various vir*EnsureACL
API's to provide the connection driver name that is causing
the failure. This should provide the context for writing the
polkit rules that would allow access via the driver.
Signed-off-by: John Ferlan
---
src/access/viraccessmanager.c | 25 +
src/rpc/gendispatch.pl| 2 +-
src/util/virerror.c | 4 ++--
3 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
index e7b5bf38da..1dfff32b9d 100644
--- a/src/access/viraccessmanager.c
+++ b/src/access/viraccessmanager.c
@@ -196,11 +196,12 @@ static void virAccessManagerDispose(void *object)
* should the admin need to debug things
*/
static int
-virAccessManagerSanitizeError(int ret)
+virAccessManagerSanitizeError(int ret,
+ const char *driverName)
{
if (ret < 0) {
virResetLastError();
-virAccessError(VIR_ERR_ACCESS_DENIED, NULL);
+virAccessError(VIR_ERR_ACCESS_DENIED, driverName, NULL);
}
return ret;
@@ -217,7 +218,7 @@ int virAccessManagerCheckConnect(virAccessManagerPtr
manager,
if (manager->drv->checkConnect)
ret = manager->drv->checkConnect(manager, driverName, perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
@@ -233,7 +234,7 @@ int virAccessManagerCheckDomain(virAccessManagerPtr manager,
if (manager->drv->checkDomain)
ret = manager->drv->checkDomain(manager, driverName, domain, perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckInterface(virAccessManagerPtr manager,
@@ -248,7 +249,7 @@ int virAccessManagerCheckInterface(virAccessManagerPtr
manager,
if (manager->drv->checkInterface)
ret = manager->drv->checkInterface(manager, driverName, iface, perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckNetwork(virAccessManagerPtr manager,
@@ -263,7 +264,7 @@ int virAccessManagerCheckNetwork(virAccessManagerPtr
manager,
if (manager->drv->checkNetwork)
ret = manager->drv->checkNetwork(manager, driverName, network, perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckNodeDevice(virAccessManagerPtr manager,
@@ -278,7 +279,7 @@ int virAccessManagerCheckNodeDevice(virAccessManagerPtr
manager,
if (manager->drv->checkNodeDevice)
ret = manager->drv->checkNodeDevice(manager, driverName, nodedev,
perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckNWFilter(virAccessManagerPtr manager,
@@ -293,7 +294,7 @@ int virAccessManagerCheckNWFilter(virAccessManagerPtr
manager,
if (manager->drv->checkNWFilter)
ret = manager->drv->checkNWFilter(manager, driverName, nwfilter, perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckNWFilterBinding(virAccessManagerPtr manager,
@@ -308,7 +309,7 @@ int
virAccessManagerCheckNWFilterBinding(virAccessManagerPtr manager,
if (manager->drv->checkNWFilterBinding)
ret = manager->drv->checkNWFilterBinding(manager, driverName, binding,
perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckSecret(virAccessManagerPtr manager,
@@ -323,7 +324,7 @@ int virAccessManagerCheckSecret(virAccessManagerPtr manager,
if (manager->drv->checkSecret)
ret = manager->drv->checkSecret(manager, driverName, secret, perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckStoragePool(virAccessManagerPtr manager,
@@ -338,7 +339,7 @@ int virAccessManagerCheckStoragePool(virAccessManagerPtr
manager,
if (manager->drv->checkStoragePool)
ret = manager->drv->checkStoragePool(manager, driverName, pool, perm);
-return virAccessManagerSanitizeError(ret);
+return virAccessManagerSanitizeError(ret, driverName);
}
int virAccessManagerCheckStorageVol(virAccessManagerPtr
[libvirt] [PATCH 0/2] Augment access denied error message and adjust polkit docs
Details in the patches John Ferlan (2): access: Modify the VIR_ERR_ACCESS_DENIED to include driverName docs: Enhance polkit documentation to describe secondary connection docs/aclpolkit.html.in| 117 ++ docs/libvirt.css | 1 + src/access/viraccessmanager.c | 25 src/rpc/gendispatch.pl| 2 +- src/util/virerror.c | 4 +- 5 files changed, 134 insertions(+), 15 deletions(-) -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ocaml PATCH] doc: invoke ocamldoc with -colorize-code
This way, the OCaml snippets are colorized. The OCaml version required is higher than the first version shipping ocamldoc with this option, so that can be done unconditionally. Signed-off-by: Pino Toscano --- Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.in b/Makefile.in index ad5a036..f119dbc 100644 --- a/Makefile.in +++ b/Makefile.in @@ -23,7 +23,7 @@ INSTALL = @INSTALL@ MAKENSIS = @MAKENSIS@ OCAMLDOC= @OCAMLDOC@ -OCAMLDOCFLAGS := -html -sort +OCAMLDOCFLAGS := -html -sort -colorize-code SUBDIRS= libvirt examples -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ocaml PATCH 3/4] build: remove config.h.in
It is generated by autoheader, no need to keep it in the sources. Signed-off-by: Pino Toscano --- .gitignore | 1 + config.h.in | 61 - 2 files changed, 1 insertion(+), 61 deletions(-) delete mode 100644 config.h.in diff --git a/.gitignore b/.gitignore index 1d9fc14..67f9134 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ core.* /autom4te.cache /config.cache /config.h +/config.h.in /config.log /config.status /configure diff --git a/config.h.in b/config.h.in deleted file mode 100644 index c0bd102..000 --- a/config.h.in +++ /dev/null @@ -1,61 +0,0 @@ -/* config.h.in. Generated from configure.ac by autoheader. */ - -/* Define to 1 if you have the header file. */ -#undef HAVE_INTTYPES_H - -/* Define to 1 if you have the `virt' library (-lvirt). */ -#undef HAVE_LIBVIRT - -/* Define to 1 if you have the header file. */ -#undef HAVE_MEMORY_H - -/* Define to 1 if you have the header file. */ -#undef HAVE_STDINT_H - -/* Define to 1 if you have the header file. */ -#undef HAVE_STDLIB_H - -/* Define to 1 if you have the header file. */ -#undef HAVE_STRINGS_H - -/* Define to 1 if you have the header file. */ -#undef HAVE_STRING_H - -/* Define to 1 if you have the header file. */ -#undef HAVE_SYS_STAT_H - -/* Define to 1 if you have the header file. */ -#undef HAVE_SYS_TYPES_H - -/* Define to 1 if you have the header file. */ -#undef HAVE_UNISTD_H - -/* Define to 1 if your C compiler doesn't accept -c and -o together. */ -#undef NO_MINUS_C_MINUS_O - -/* Define to the address where bug reports for this package should be sent. */ -#undef PACKAGE_BUGREPORT - -/* Define to the full name of this package. */ -#undef PACKAGE_NAME - -/* Define to the full name and version of this package. */ -#undef PACKAGE_STRING - -/* Define to the one symbol short name of this package. */ -#undef PACKAGE_TARNAME - -/* Define to the home page for this package. */ -#undef PACKAGE_URL - -/* Define to the version of this package. */ -#undef PACKAGE_VERSION - -/* Define to 1 if the C compiler supports function prototypes. */ -#undef PROTOTYPES - -/* Define to 1 if you have the ANSI C header files. */ -#undef STDC_HEADERS - -/* Define like PROTOTYPES; this can be used by system headers. */ -#undef __PROTOTYPES -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ocaml PATCH 0/4] Misc build improvements
A round of few build system improvements: - use the OCaml build macros from libguestfs - remove generated stuff - use pkg-config to find & use libvirt Pino Toscano (4): build: move OCaml macros to a m4 subdir build: sync OCaml macros from libguestfs build: remove config.h.in build: use pkg-config to find libvirt .gitignore | 2 + aclocal.m4 | 170 -- config.h.in | 61 - configure.ac| 26 +- libvirt/Makefile.in | 14 +-- m4/ocaml.m4 | 217 6 files changed, 232 insertions(+), 258 deletions(-) delete mode 100644 aclocal.m4 delete mode 100644 config.h.in create mode 100644 m4/ocaml.m4 -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH] doc: invoke ocamldoc with -colorize-code
On Mon, Oct 15, 2018 at 04:07:01PM +0200, Pino Toscano wrote: > This way, the OCaml snippets are colorized. > > The OCaml version required is higher than the first version shipping > ocamldoc with this option, so that can be done unconditionally. > > Signed-off-by: Pino Toscano > --- > Makefile.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile.in b/Makefile.in > index ad5a036..f119dbc 100644 > --- a/Makefile.in > +++ b/Makefile.in > @@ -23,7 +23,7 @@ INSTALL = @INSTALL@ > MAKENSIS = @MAKENSIS@ > > OCAMLDOC= @OCAMLDOC@ > -OCAMLDOCFLAGS:= -html -sort > +OCAMLDOCFLAGS:= -html -sort -colorize-code > > SUBDIRS = libvirt examples Trivial improvement, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 0/4] Misc build improvements
On Mon, Oct 15, 2018 at 04:02:31PM +0200, Pino Toscano wrote: > A round of few build system improvements: > - use the OCaml build macros from libguestfs > - remove generated stuff > - use pkg-config to find & use libvirt > > Pino Toscano (4): > build: move OCaml macros to a m4 subdir > build: sync OCaml macros from libguestfs > build: remove config.h.in > build: use pkg-config to find libvirt > > .gitignore | 2 + > aclocal.m4 | 170 -- > config.h.in | 61 - > configure.ac| 26 +- > libvirt/Makefile.in | 14 +-- > m4/ocaml.m4 | 217 > 6 files changed, 232 insertions(+), 258 deletions(-) > delete mode 100644 aclocal.m4 > delete mode 100644 config.h.in > create mode 100644 m4/ocaml.m4 Looks fine, ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] conf: Forbid hugepages and
On 10/15/2018 03:17 PM, Daniel P. Berrangé wrote: > On Mon, Oct 15, 2018 at 02:39:26PM +0200, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1633562 >> >> Under we allow users to configure various memory >> backing related knobs. However, there are some combinations that >> make no sense. For instance, requesting hugepages and file >> allocation at the same time. Forbid this configuration then. > > The huge pages we allocate come from a file in /dev/hugepages, > so this description doesn't really make sense. Sure, in the end the mmap() that qemu calls is a file somewhere under /dev/hugepages (btw terrible, really terrible path for hugetlbfs mount point because it suggests hugepages are devices). But is usually used to put domain RAM under /var/lib/libvirt/ram/.. (and to enforce memory-backing-file). > > IIUC, what's actually happening is that you're trying to avoid > a historical bug. More or less. This basically a result of 992bf863fcc. Anyway, might as well do nothing on this front :-) Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ocaml PATCH 4/4] build: use pkg-config to find libvirt
Rely on pkg-config to detect libvirt, and use its variables to locate it. The version required is taken from the API documentation. Signed-off-by: Pino Toscano --- configure.ac| 20 ++-- libvirt/Makefile.in | 14 -- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/configure.ac b/configure.ac index 11ff2bf..7d923bf 100644 --- a/configure.ac +++ b/configure.ac @@ -55,24 +55,8 @@ if test "x$PERL" = "xno"; then fi dnl Check for libvirt development environment. -AC_ARG_WITH(libvirt, - AC_HELP_STRING([--with-libvirt=PATH],[Set path to installed libvirt]), - [if test "x$withval" != "x"; then - CFLAGS="$CFLAGS -I$withval/include" - LDFLAGS="$LDFLAGS -L$withval/lib" -fi - ]) -AC_CHECK_LIB(virt,virConnectOpen, - [], - AC_MSG_ERROR([You must install libvirt library])) -AC_CHECK_HEADER([libvirt/libvirt.h], - [], - AC_MSG_ERROR([You must install libvirt development package])) - -dnl We also use -AC_CHECK_HEADER([libvirt/virterror.h], - [], - AC_MSG_ERROR([You must install libvirt development package])) +PKG_PROG_PKG_CONFIG +PKG_CHECK_MODULES([LIBVIRT], [libvirt >= 1.0.2]) dnl Check for basic OCaml environment & findlib. AC_PROG_OCAML diff --git a/libvirt/Makefile.in b/libvirt/Makefile.in index faca5ee..3a68aed 100644 --- a/libvirt/Makefile.in +++ b/libvirt/Makefile.in @@ -18,10 +18,12 @@ WIN32 = @WIN32@ CFLAGS = @CFLAGS@ \ + @LIBVIRT_CFLAGS@ \ -I.. \ -I"$(shell ocamlc -where)" \ @DEBUG@ @WARNINGS@ @CFLAGS_FPIC@ -LDFLAGS= @LDFLAGS@ +LDFLAGS= @LDFLAGS@ \ + @LIBVIRT_LIBS@ # -L"$(shell ocamlc -where)" OCAMLC = @OCAMLC@ @@ -62,10 +64,10 @@ OPTOBJS := libvirt.cmx libvirt_version.cmx ifneq ($(OCAMLMKLIB),) # Good, we can just use ocamlmklib mllibvirt.cma: libvirt_c.o $(COBJS) - $(OCAMLMKLIB) -o mllibvirt $^ $(LDFLAGS) -lvirt + $(OCAMLMKLIB) -o mllibvirt $^ $(LDFLAGS) mllibvirt.cmxa: libvirt_c.o $(OPTOBJS) - $(OCAMLMKLIB) -o mllibvirt $^ $(LDFLAGS) -lvirt + $(OCAMLMKLIB) -o mllibvirt $^ $(LDFLAGS) else ifeq ($(WIN32),yes) @@ -74,15 +76,15 @@ ifeq ($(WIN32),yes) mllibvirt.cma: dllmllibvirt.dll libmllibvirt.a $(COBJS) $(OCAMLC) -a -linkall -o $@ $(COBJS) \ - -dllib -lmllibvirt -cclib -lmllibvirt -cclib "$(LDFLAGS) -lvirt" + -dllib -lmllibvirt -cclib -lmllibvirt -cclib "$(LDFLAGS)" mllibvirt.cmxa: libmllibvirt.a $(OPTOBJS) $(OCAMLOPT) -a -linkall -o $@ $(OPTOBJS) \ - -cclib -lmllibvirt -cclib "$(LDFLAGS) -lvirt" + -cclib -lmllibvirt -cclib "$(LDFLAGS)" dllmllibvirt.dll: libvirt_c.o $(CC) -shared -o $@ $^ \ - $(LDFLAGS) "$(shell ocamlc -where)"/ocamlrun.a -lvirt + $(LDFLAGS) "$(shell ocamlc -where)"/ocamlrun.a libmllibvirt.a: libvirt_c.o ar rc $@ $^ -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ocaml PATCH 2/4] build: sync OCaml macros from libguestfs
Less old version, and work better with newer versions of autoconf. Adapt configure.ac to it: - use the different variable names set by AC_CHECK_OCAML_PKG - remove the comment about using ocamlfind, as it is what the new macros do already Signed-off-by: Pino Toscano --- configure.ac | 5 +- m4/ocaml.m4 | 331 +-- 2 files changed, 191 insertions(+), 145 deletions(-) diff --git a/configure.ac b/configure.ac index dcb4ae7..11ff2bf 100644 --- a/configure.ac +++ b/configure.ac @@ -82,11 +82,10 @@ if test "x$OCAMLFIND" = "x"; then AC_MSG_ERROR([OCaml findlib is required]) fi -dnl Use ocamlfind to find the required packages ... - dnl Check for required OCaml packages. +OCAML_PKG_unix=no AC_CHECK_OCAML_PKG(unix) -if test "x$pkg_unix" != "xyes"; then +if test "x$OCAML_PKG_unix" = "xno"; then AC_MSG_ERROR([Cannot find required OCaml package 'unix']) fi diff --git a/m4/ocaml.m4 b/m4/ocaml.m4 index 38ad15f..fddd6a0 100644 --- a/m4/ocaml.m4 +++ b/m4/ocaml.m4 @@ -1,170 +1,217 @@ dnl autoconf macros for OCaml -dnl by Olivier Andrieu -dnl modified by Richard W.M. Jones -dnl from a configure.in by Jean-Christophe Filli�tre, -dnl from a first script by Georges Mariano dnl -dnl defines AC_PROG_OCAML that will check the OCaml compiler -dnl and set the following variables : -dnl OCAMLC"ocamlc" if present in the path, or a failure -dnl or "ocamlc.opt" if present with same version number as ocamlc -dnl OCAMLOPT "ocamlopt" (or "ocamlopt.opt" if present), or "no" -dnl OCAMLBEST either "byte" if no native compiler was found, -dnl or "opt" otherwise -dnl OCAMLDEP "ocamldep" -dnl OCAMLLIB the path to the ocaml standard library -dnl OCAMLVERSION the ocaml version number -AC_DEFUN(AC_PROG_OCAML, +dnl Copyright © 2009 Richard W.M. Jones +dnl Copyright © 2009 Stefano Zacchiroli +dnl Copyright © 2000-2005 Olivier Andrieu +dnl Copyright © 2000-2005 Jean-Christophe Filliâtre +dnl Copyright © 2000-2005 Georges Mariano +dnl +dnl For documentation, please read the ocaml.m4 man page. + +AC_DEFUN([AC_PROG_OCAML], [dnl -# checking for ocamlc -AC_CHECK_PROG(OCAMLC,ocamlc,ocamlc,AC_MSG_ERROR(Cannot find ocamlc.)) -OCAMLVERSION=`$OCAMLC -v | sed -n -e 's|.*version* *\(.*\)$|\1|p' ` -AC_MSG_RESULT(OCaml version is $OCAMLVERSION) -OCAMLLIB=`$OCAMLC -where 2>/dev/null || $OCAMLC -v|tail -1|cut -d ' ' -f 4` -AC_MSG_RESULT(OCaml library path is $OCAMLLIB) -# checking for ocamlopt -AC_CHECK_PROG(OCAMLOPT,ocamlopt,ocamlopt) -OCAMLBEST=byte -if test -z "$OCAMLOPT"; then - AC_MSG_WARN(Cannot find ocamlopt; bytecode compilation only.) -else - TMPVERSION=`$OCAMLOPT -v | sed -n -e 's|.*version* *\(.*\)$|\1|p' ` - if test "$TMPVERSION" != "$OCAMLVERSION" ; then - AC_MSG_RESULT(versions differs from ocamlc; ocamlopt discarded.) - unset OCAMLOPT - else - OCAMLBEST=opt - fi -fi -# checking for ocamlc.opt -AC_CHECK_PROG(OCAMLCDOTOPT,ocamlc.opt,ocamlc.opt) -if test -z "$OCAMLCDOTOPT"; then - TMPVERSION=`$OCAMLCDOTOPT -v | sed -n -e 's|.*version* *\(.*\)$|\1|p' ` - if test "$TMPVERSION" != "$OCAMLVERSION" ; then - AC_MSG_RESULT(versions differs from ocamlc; ocamlc.opt discarded.) - else - OCAMLC=$OCAMLCDOTOPT - fi -fi -# checking for ocamlopt.opt -if test "$OCAMLOPT" ; then -AC_CHECK_PROG(OCAMLOPTDOTOPT,ocamlopt.opt,ocamlopt.opt) -if test "$OCAMLOPTDOTOPT"; then - TMPVER=`$OCAMLOPTDOTOPT -v | sed -n -e 's|.*version* *\(.*\)$|\1|p' ` - if test "$TMPVER" != "$OCAMLVERSION" ; then - AC_MSG_RESULT(version differs from ocamlc; ocamlopt.opt discarded.) - else - OCAMLOPT=$OCAMLOPTDOTOPT - fi -fi -fi -# checking for ocamldep -AC_CHECK_PROG(OCAMLDEP,ocamldep,ocamldep,AC_MSG_ERROR(Cannot find ocamldep.)) - -#checking for ocamlmktop -AC_CHECK_PROG(OCAMLMKTOP,ocamlmktop,ocamlmktop, AC_MSG_WARN(Cannot find ocamlmktop.)) -#checking for ocamlmklib -AC_CHECK_PROG(OCAMLMKLIB,ocamlmklib,ocamlmklib, AC_MSG_WARN(Cannot find ocamlmklib.)) -# checking for ocamldoc -AC_CHECK_PROG(OCAMLDOC,ocamldoc,ocamldoc, AC_MSG_WARN(Cannot find ocamldoc.)) - - -AC_SUBST(OCAMLC) -AC_SUBST(OCAMLOPT) -AC_SUBST(OCAMLDEP) -AC_SUBST(OCAMLBEST) -AC_SUBST(OCAMLVERSION) -AC_SUBST(OCAMLLIB) -AC_SUBST(OCAMLMKLIB) -AC_SUBST(OCAMLDOC) + # checking for ocamlc + AC_CHECK_TOOL([OCAMLC],[ocamlc],[no]) + + if test "$OCAMLC" != "no"; then + OCAMLVERSION=`$OCAMLC -v | sed -n -e 's|.*version* *\(.*\)$|\1|p'` + AC_MSG_RESULT([OCaml version is $OCAMLVERSION]) + OCAMLLIB=`$OCAMLC -where 2>/dev/null || $OCAMLC -v|tail -1|cut -d ' ' -f 4` + AC_MSG_RESULT([OCaml library path is $OCAMLLIB]) + + AC_SUBST([OCAMLVERSION]) + AC_SUBST([OCAMLLIB]) + + # checking for ocamlopt + AC_CHECK_TOOL([OCAMLOPT],[ocamlopt],[no]) + OCAMLBEST=byte + if test "$OCAMLOPT" = "no"; then +
[libvirt] [ocaml PATCH 1/4] build: move OCaml macros to a m4 subdir
This way they will not be overwritten when aclocal.m4 is changed. Signed-off-by: Pino Toscano --- .gitignore| 1 + configure.ac | 1 + aclocal.m4 => m4/ocaml.m4 | 0 3 files changed, 2 insertions(+) rename aclocal.m4 => m4/ocaml.m4 (100%) diff --git a/.gitignore b/.gitignore index 32409ae..1d9fc14 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ Makefile core core.* /META +/aclocal.m4 /autom4te.cache /config.cache /config.h diff --git a/configure.ac b/configure.ac index 66f0cf2..dcb4ae7 100644 --- a/configure.ac +++ b/configure.ac @@ -18,6 +18,7 @@ dnl Process this file with autoconf to produce a configure script. AC_INIT(ocaml-libvirt,0.6.1.4) +AC_CONFIG_MACRO_DIRS([m4]) dnl Check for basic C environment. AC_PROG_CC diff --git a/aclocal.m4 b/m4/ocaml.m4 similarity index 100% rename from aclocal.m4 rename to m4/ocaml.m4 -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] conf: Forbid hugepages and
On Mon, Oct 15, 2018 at 02:39:26PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1633562
>
> Under we allow users to configure various memory
> backing related knobs. However, there are some combinations that
> make no sense. For instance, requesting hugepages and file
> allocation at the same time. Forbid this configuration then.
The huge pages we allocate come from a file in /dev/hugepages,
so this description doesn't really make sense.
IIUC, what's actually happening is that you're trying to avoid
a historical bug.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/domain_conf.c | 16 ++--
> tests/qemuxml2argvdata/hugepages-memaccess2.xml | 1 -
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9911d56130..ef1d5caa1c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4041,7 +4041,7 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
> }
>
>
> -static void
> +static int
> virDomainDefPostParseMemtune(virDomainDefPtr def)
> {
> size_t i;
> @@ -4063,6 +4063,17 @@ virDomainDefPostParseMemtune(virDomainDefPtr def)
> }
> }
> }
> +
> +if (def->mem.nhugepages &&
> +def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
> +def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
> +virReportError(VIR_ERR_XML_ERROR,
> + _("unsupported combination of hugepages and source
> type %s"),
> + virDomainMemorySourceTypeToString(def->mem.source));
> +return -1;
> +}
> +
> +return 0;
> }
>
>
> @@ -5131,7 +5142,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
> if (virDomainDefPostParseMemory(def, data->parseFlags) < 0)
> return -1;
>
> -virDomainDefPostParseMemtune(def);
> +if (virDomainDefPostParseMemtune(def) < 0)
> +return -1;
>
> if (virDomainDefRejectDuplicateControllers(def) < 0)
> return -1;
> diff --git a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
> b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
> index 205f9efd92..e7f60291be 100644
> --- a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
> +++ b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
> @@ -8,7 +8,6 @@
>
>
>
> -
>
>
>4
> --
> 2.18.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PULL 0/2] Ui2 20181012 patches
On 12 October 2018 at 15:05, Gerd Hoffmann wrote: > The following changes since commit 69ac8c4cb93f2685839ff7b857cef306b388ff3c: > > Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20181012' into > staging (2018-10-12 12:40:04 +0100) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/ui2-20181012-pull-request > > for you to fetch changes up to 58296cb61866195297510e946a51acc5f0b9639e: > > ui: increase min required GTK3 version to 3.14.0 (2018-10-12 15:22:18 +0200) > > > ui: drop gtk2 support. > > > > Daniel P. Berrangé (2): > ui: remove support for GTK2 in favour of GTK3 > ui: increase min required GTK3 version to 3.14.0 > > configure| 51 ++--- > include/ui/gtk.h | 9 --- > ui/gtk-egl.c | 10 +-- > ui/gtk.c | 202 > --- > qemu-deprecated.texi | 7 -- > 5 files changed, 26 insertions(+), 253 deletions(-) Applied, thanks. -- PMM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] conf: Forbid hugepages and
https://bugzilla.redhat.com/show_bug.cgi?id=1633562
Under we allow users to configure various memory
backing related knobs. However, there are some combinations that
make no sense. For instance, requesting hugepages and 'ondemand'
allocation at the same time. Forbid this configuration then.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ef1d5caa1c..53cb4209d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4064,13 +4064,22 @@ virDomainDefPostParseMemtune(virDomainDefPtr def)
}
}
-if (def->mem.nhugepages &&
-def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
-def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
-virReportError(VIR_ERR_XML_ERROR,
- _("unsupported combination of hugepages and source type
%s"),
- virDomainMemorySourceTypeToString(def->mem.source));
-return -1;
+if (def->mem.nhugepages) {
+if (def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
+def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported combination of hugepages and source
type %s"),
+ virDomainMemorySourceTypeToString(def->mem.source));
+return -1;
+}
+
+if (def->mem.allocation != VIR_DOMAIN_MEMORY_ALLOCATION_NONE &&
+def->mem.allocation != VIR_DOMAIN_MEMORY_ALLOCATION_IMMEDIATE) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported combination of hugepages and
allocation %s"),
+
virDomainMemoryAllocationTypeToString(def->mem.allocation));
+return -1;
+}
}
return 0;
--
2.18.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] conf: Forbid hugepages and
https://bugzilla.redhat.com/show_bug.cgi?id=1633562
Under we allow users to configure various memory
backing related knobs. However, there are some combinations that
make no sense. For instance, requesting hugepages and file
allocation at the same time. Forbid this configuration then.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 16 ++--
tests/qemuxml2argvdata/hugepages-memaccess2.xml | 1 -
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9911d56130..ef1d5caa1c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4041,7 +4041,7 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
}
-static void
+static int
virDomainDefPostParseMemtune(virDomainDefPtr def)
{
size_t i;
@@ -4063,6 +4063,17 @@ virDomainDefPostParseMemtune(virDomainDefPtr def)
}
}
}
+
+if (def->mem.nhugepages &&
+def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
+def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported combination of hugepages and source type
%s"),
+ virDomainMemorySourceTypeToString(def->mem.source));
+return -1;
+}
+
+return 0;
}
@@ -5131,7 +5142,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
if (virDomainDefPostParseMemory(def, data->parseFlags) < 0)
return -1;
-virDomainDefPostParseMemtune(def);
+if (virDomainDefPostParseMemtune(def) < 0)
+return -1;
if (virDomainDefRejectDuplicateControllers(def) < 0)
return -1;
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
index 205f9efd92..e7f60291be 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
+++ b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
@@ -8,7 +8,6 @@
-
4
--
2.18.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] conf: Forbid some nonsensical combinations under
*** BLURB HERE *** Michal Prívozník (2): conf: Forbid hugepages and conf: Forbid hugepages and src/conf/domain_conf.c| 25 +-- .../qemuxml2argvdata/hugepages-memaccess2.xml | 1 - 2 files changed, 23 insertions(+), 3 deletions(-) -- 2.18.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] version: Add ParseVersion and a Version struct
Make it easier to convert version integers to the more human-readable
major.minor.release format.
---
connect.go | 8
version.go | 52 ++
version_test.go | 64 +
3 files changed, 124 insertions(+)
create mode 100644 version.go
create mode 100644 version_test.go
diff --git a/connect.go b/connect.go
index 8cc7cc7..fac45da 100644
--- a/connect.go
+++ b/connect.go
@@ -46,6 +46,7 @@ func init() {
}
const (
+ // VERSION_NUMBER is the version of the linked libvirt.
VERSION_NUMBER = uint32(C.LIBVIR_VERSION_NUMBER)
)
@@ -306,7 +307,9 @@ func releaseConnectionData(c *Connect) {
delete(connections, c.ptr)
}
+// GetVersion returns the version of the linked libvirt.
// See also https://libvirt.org/html/libvirt-libvirt-host.html#virGetVersion
+// and ParseVersion.
func GetVersion() (uint32, error) {
var version C.ulong
var err C.virError
@@ -540,7 +543,10 @@ func (c *Connect) GetHostname() (string, error) {
return hostname, nil
}
+// GetLibVersion returns the version of libvirt used by the daemon
+// running on the connected host.
// See also
https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetLibVersion
+// and ParseVersion.
func (c *Connect) GetLibVersion() (uint32, error) {
var version C.ulong
var err C.virError
@@ -2272,7 +2278,9 @@ func (c *Connect) GetDomainCapabilities(emulatorbin
string, arch string, machine
return C.GoString(ret), nil
}
+// GetVersion returns the hypervisor version.
// See also
https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetVersion
+// and ParseVersion.
func (c *Connect) GetVersion() (uint32, error) {
var hvVer C.ulong
var err C.virError
diff --git a/version.go b/version.go
new file mode 100644
index 000..1a07fa3
--- /dev/null
+++ b/version.go
@@ -0,0 +1,52 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ */
+
+package libvirt
+
+import (
+ "fmt"
+)
+
+// Version represents the libvirt version.
+// See also https://libvirt.org/html/libvirt-libvirt-host.html#virGetVersion
+type Version struct {
+ Major uint32
+ Minor uint32
+ Release uint32
+}
+
+// String returns the "major.minor.release" version string.
+func (v *Version) String() string {
+ return fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Release)
+}
+
+// ParseVersion converts a version integer to a structured Version instance.
+// See also https://libvirt.org/html/libvirt-libvirt-host.html#virGetVersion
+func ParseVersion(version uint32) *Version {
+ return {
+ Major: version / 100,
+ Minor: (version / 1000) % 1000,
+ Release: version % 1000,
+ }
+}
diff --git a/version_test.go b/version_test.go
new file mode 100644
index 000..ecafe5d
--- /dev/null
+++ b/version_test.go
@@ -0,0 +1,64 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
[libvirt] [PATCH 2/3] qemu: vfio-ap device support
Adjusting domain format documentation, adding device address
support and adding command line generation for vfio-ap.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
docs/formatdomain.html.in | 3 ++-
docs/schemas/domaincommon.rng | 1 +
src/qemu/qemu_command.c| 8
src/qemu/qemu_domain_address.c | 4
src/util/virmdev.c | 3 ++-
src/util/virmdev.h | 1 +
6 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8189959773..269741a690 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4616,8 +4616,9 @@
For mediated devices (Since 3.2.0)
the model attribute specifies the device API which
determines how the host's vfio driver will expose the device to the
- guest. Currently, model='vfio-pci' and
+ guest. Currently, model='vfio-pci',
model='vfio-ccw' (Since
4.4.0)
+ and model='vfio-ap' (Since
4.9.0)
is supported. MDEV section
provides more information about mediated devices as well as how to
create mediated devices on the host.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 099a949cf8..b9ac5df479 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4618,6 +4618,7 @@
vfio-pci
vfio-ccw
+vfio-ap
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 269276f2f9..fc5ab8b760 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5476,6 +5476,14 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
return -1;
}
break;
+case VIR_MDEV_MODEL_TYPE_VFIO_AP:
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_AP)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("VFIO AP device assignment is not "
+ "supported by this version of QEMU"));
+return -1;
+}
+break;
case VIR_MDEV_MODEL_TYPE_LAST:
default:
virReportEnumRangeError(virMediatedDeviceModelType,
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 8a8764cff5..24dd7c1a58 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -294,6 +294,10 @@ qemuDomainPrimeVfioDeviceAddresses(virDomainDefPtr def,
subsys->u.mdev.model == VIR_MDEV_MODEL_TYPE_VFIO_CCW &&
def->hostdevs[i]->info->type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
def->hostdevs[i]->info->type = type;
+
+if (virHostdevIsMdevDevice(def->hostdevs[i]) &&
+subsys->u.mdev.model == VIR_MDEV_MODEL_TYPE_VFIO_AP)
+def->hostdevs[i]->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
}
}
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 10a2b08337..3e11e38802 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -48,7 +48,8 @@ struct _virMediatedDeviceList {
VIR_ENUM_IMPL(virMediatedDeviceModel, VIR_MDEV_MODEL_TYPE_LAST,
"vfio-pci",
- "vfio-ccw")
+ "vfio-ccw",
+ "vfio-ap")
static virClassPtr virMediatedDeviceListClass;
diff --git a/src/util/virmdev.h b/src/util/virmdev.h
index 7c93c4d390..c856ff5bdb 100644
--- a/src/util/virmdev.h
+++ b/src/util/virmdev.h
@@ -27,6 +27,7 @@
typedef enum {
VIR_MDEV_MODEL_TYPE_VFIO_PCI = 0,
VIR_MDEV_MODEL_TYPE_VFIO_CCW = 1,
+VIR_MDEV_MODEL_TYPE_VFIO_AP = 2,
VIR_MDEV_MODEL_TYPE_LAST
} virMediatedDeviceModelType;
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] qemu: guest dedicated crypto adapters
This patch series introduces initial libvirt support for guest dedicated crypto adapters on S390. It allows to specify a vfio-ap mediated device in a domain. Extensive documentation about AP is available in patch 6 of the QEMU patch series. KVM/kernel: guest dedicated crypto adapters https://lkml.org/lkml/2018/9/26/25 QEMU: s390x: vfio-ap: guest dedicated crypto adapters https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg01993.html The qemu patch series has been included into master. https://git.qemu.org/?p=qemu.git;a=commit;h=69ac8c4cb93f2685839ff7b857cef306b388ff3c Boris Fiuczynski (3): qemu: add vfio-ap capability qemu: vfio-ap device support news: Update news for vfio-ap support docs/formatdomain.html.in | 3 ++- docs/news.xml | 9 + docs/schemas/domaincommon.rng | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 8 src/qemu/qemu_domain_address.c | 4 src/util/virmdev.c | 3 ++- src/util/virmdev.h | 1 + 9 files changed, 30 insertions(+), 2 deletions(-) -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: add vfio-ap capability
Introduce vfio-ap capability.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e228f52ec0..2ca5af3297 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -508,6 +508,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
/* 315 */
"vfio-pci.display",
"blockdev",
+ "vfio-ap",
);
@@ -1092,6 +1093,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "vhost-vsock-device", QEMU_CAPS_DEVICE_VHOST_VSOCK },
{ "mch", QEMU_CAPS_DEVICE_MCH },
{ "sev-guest", QEMU_CAPS_SEV_GUEST },
+{ "vfio-ap", QEMU_CAPS_DEVICE_VFIO_AP },
};
static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 934620ed31..6bb9a2c8f0 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -492,6 +492,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 315 */
QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */
QEMU_CAPS_BLOCKDEV, /* -blockdev and blockdev-add are supported */
+QEMU_CAPS_DEVICE_VFIO_AP, /* -device vfio-ap */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] news: Update news for vfio-ap support
Signed-off-by: Boris Fiuczynski --- docs/news.xml | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index dc08c96352..e5476a3332 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,15 @@ + + + qemu: Add vfio AP support + + + The QEMU driver now has support to passthrough adjunct processors + into QEMU guests on S390. + + -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 10/19] cdtil: Introduce default monitor
On 10/12/2018 11:18 PM, John Ferlan wrote: On 10/11/18 8:02 AM, Wang, Huaqiang wrote: Answers refined. On 10/11/2018 3:14 AM, John Ferlan wrote: On 10/9/18 6:30 AM, Wang Huaqiang wrote: In resctrl file system, more than one monitoring groups could be created within one allocation group, along with the creation of allocation group, a monitoring group is created at the same, which monitors the resource utilization information of whole allocation group. This patch is introducing the concept of default monitor, which represents the particular monitoring group that created along with the creation of allocation group. Default monitor shares the common 'vcpu' list with the allocation. Signed-off-by: Wang Huaqiang --- src/libvirt_private.syms | 1 + src/util/virresctrl.c| 23 +++ src/util/virresctrl.h| 2 ++ 3 files changed, 26 insertions(+) I assume the two responses to my review are very similar, but I'll use this second one since it's timewise later... diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c93d19f..4b22ed4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2689,6 +2689,7 @@ virResctrlMonitorGetID; virResctrlMonitorNew; virResctrlMonitorRemove; virResctrlMonitorSetCacheLevel; +virResctrlMonitorSetDefault; virResctrlMonitorSetID; [...] + * a monitoring group is created at the same, which monitors the resource + * utilization information of whole allocation group. Reword - it's a bit redundant Rewording like these: * There are two type of monitors, the default monitor and the non-default * monitor. Within one allocation, up to one default monitor and more than * one non-default monitors could be created. * * The flag 'default_monitor' defined in structure virResctrlMonitor denotes * if a monitor is the default one or not. Starting with "Within one allocation,..." - doesn't make much sense to me as a reader, sorry. + * A virResctrlMonitor with @default_monitor marked as 'true' is representing + * the monitoring group created along with the creation of allocation group. Well I'm a bit lost, but let's see what happens. I'm not sure what you're trying to delineate here. There is the creation of an resctrl->alloc when a is found by no is found. Thus, we create an empty (one with no elements). To me that's a default environment. Re-read above paragraph, you have pointed out very clearly about your point, and now I finally catch up with you ... and the default monitor could be simplified: Before there is a pointer point to 'allocation', which is @resctrl->alloc in dom_conf.c or @monitor->alloc in virresctrl.c, the 'default' case could be determined by checking if @alloc is empty pointer or not. Then there is no need for 'virResctrlMonitorSetDefault' and '@monitor->default_monitor' variable. Will remove the concept of 'default monitor' and related data field in virResctrlMonitor as well as the API(s). (mainly virResctrlMonitorSetDefault). 'Default allocation' concept related wording will be removed too. And there are not too much data field and API(s) related to default allocation, and I will check my code to remove all stuff for it. Do we have any gap about this? I assume a similar paradigm could exist if there was or wasn't a element... Make you confused, my bad. I was trying to introducing the situation of '/sys/fs/resctrl' filesystem and what I was done here at the same time. Regarding the XML layout for default monitor, following XML lines represents a default monitor, the key rule is the default monitor's entry has the same 'vcpus' setting as . and following example illustrates a with two monitors, one of them is a default monitor This gets tagged as a default monitor just because the vcpus match? Yes. and this is not a default monitor because the vcpus don't match Yes. Particularly, following XML layout doesn't define any monitor or allocation. Following XML lines does not have any effect, and does not indicate an error. or I understand that, but that wasn't my point. If someone modified their XML and added just that, then add the resctrl->alloc as soon as you see it. Then when you see a , that gets added to some cache list. When you see a that gets added to some monitor list. The whole concept of calling some a default something just isn't working for me. It's a cache or monitor for either all or some subset of the vcpus for the cachetune (or resctrl->alloc). Nothing more, nothing less. Again, I'll remove the default concepts that I tried introduced before. No concept for 'default allocation' and 'default monitor' will be removed. The difference between my called 'default monitor' and 'non-default monitor' is tiny, and could be handled with a simple judgement. (For a monitor whether is a default one or not could be found out by checking associated @alloc pointer.) */ struct
Re: [libvirt] [RFC 0/7] Warn at runtime when deprecated features are used
On Sun, Oct 14, 2018 at 01:41:27PM +0200, Andrea Bolognani wrote: > On Fri, 2018-10-12 at 16:21 +0100, Daniel P. Berrangé wrote: > > On Wed, Oct 10, 2018 at 01:09:50PM +0200, Andrea Bolognani wrote: > > > So once we have these changes in place, command line users can be > > > pretty much completely isolated from libvirt defaults, just like > > > virt-manager and oVirt and Nova users. Then it will be up to us > > > to actually advertise these alternatives and push users away from > > > virsh[1] and towards them. > > > > > > I wonder if showing a message suggesting to use virt-xml instead > > > when 'virsh edit' or 'virsh attach-device' are called would be > > > considered acceptable at that point? > > > > Depends what you mean by showing a message ? I'd be fine with the > > virsh man page referring people to virt-xml as a companion tool. > > > > I would certainly not expect invokation of 'virsh edit' to print > > any text on the console, as it will always be valid to want to > > use "virsh edit", "virsh atach-device" or any other command > > precisely because they are an almost direct passthrough to the > > libvirt API without trying to inject clever logic of their own. > > Okay, let's forget the runtime messages then: we can mention > virt-xml (and virt-install) in the documentation, write blog posts > about them, and the like. > > Does the rest of the plan look reasonable to you? Sure, I'm fine with docs updates. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter: intantiate filters on loading driver
Before using filters binding filters instantiation was done by hypervisors drivers initialization code (qemu was the only such hypervisor). Now qemu reconnection done supposes it should be done by nwfilter driver probably. Let's add this missing step. Signed-off-by: Nikolay Shirokovskiy --- src/nwfilter/nwfilter_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 1ee5162..1ab906f 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -264,6 +264,9 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterBindingObjListLoadAllConfigs(driver->bindings, driver->bindingDir) < 0) goto error; +if (virNWFilterBuildAll(driver, false) < 0) +goto error; + nwfilterDriverUnlock(); return 0; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
在 2018/10/15 下午2:59, Andrea Bolognani 写道: On Mon, 2018-10-15 at 09:31 +0800, Yi Min Zhao wrote: 在 2018/10/11 下午9:08, Andrea Bolognani 写道: Forgot to mention: it would be really nice if you added a negative test case showing that using zPCI addresses on eg. x86 will result in a validation error. OK. Let me have a try. Haven't added this kind of test before. It's very easy: just use DO_TEST_PARSE_ERROR() or DO_TEST_FAILURE() instead of DO_TEST() :) Yes, I have found sample code. Thanks! -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 09/19] util: Add more interfaces for resctrl monitor
On 10/12/2018 10:40 PM, John Ferlan wrote:
[...]
virResctrlMonitorCreate(virResctrlAllocPtr alloc,
virResctrlMonitorPtr monitor,
const char *machinename)
@@ -2534,3 +2565,177 @@ virResctrlMonitorCreate(virResctrlAllocPtr alloc,
virResctrlUnlock(lockfd);
return ret;
}
+
+
+int
The eventual only caller never checks status...
That's true, and I noticed that. The back-end logic is copied from
virResctrlAllocRemove.
Understood, but that doesn't check status either. Maybe it needs to
change to a void since it uses VIR_ERROR.
Maybe with a change of removing the following
virReportSystemError lines.
+virResctrlMonitorRemove(virResctrlMonitorPtr monitor)
+{
+int ret = 0;
+
So unlike Alloc, @monitor cannot be NULL...
@monitor is not allowed to be NULL. This is guaranteed by the caller.
+if (!monitor->path)
+return 0;
+
+VIR_DEBUG("Removing resctrl monitor%s", monitor->path);
+if (rmdir(monitor->path) != 0 && errno != ENOENT) {
+virReportSystemError(errno,
+ _("Unable to remove %s (%d)"),
+ monitor->path, errno);
+ret = -errno;
+VIR_ERROR(_("Unable to remove %s (%d)"), monitor->path, errno);
Either virReportSystemError if you're going to handle the returned
status or VIR_ERROR if you're not (and are just ignoring), but not both.
I would like to remove the virReportSystemError lines and keep the VIR_ERROR
line.
I can agree to that along with it being void since it's a best effort.
+}
+
+return ret;
+}
+
+
+int
+virResctrlMonitorSetCacheLevel(virResctrlMonitorPtr monitor,
+ unsigned int level)
+{
+/* Only supports cache level 3 CMT */
+if (level != 3) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Invalid resctrl monitor cache level"));
+return -1;
+}
+
+monitor->cache_level = level;
+
+return 0;
+}
+
+unsigned int
+virResctrlMonitorGetCacheLevel(virResctrlMonitorPtr monitor)
+{
+return monitor->cache_level;
+}
Based on usage, maybe we should just give up on API's like this. Create
a VIR_RESCTRL_MONITOR_CACHE_LEVEL (or something like it)... Then use it
at least for now when reading/supplying the level. Thus we leave it to
future developer to create the API's and handle the new levels...
If when we Parse we don't find the constant, then error.
Do you mean removing the 'virResctrlMonitorSetCacheLevel'
and 'virResctrlMonitorGetCacheLevel' two functions from here, and processing
the monitor cache level information directory in domain_conf.c during XML
parsing and format process.
Yeah, I think I've come to that conclusion. In the Parse code you'd
still parse the value, but then compare against the constant. In the
Format code, you can just format what you have. Whomever creates a
different level in the future can have the fun of managing range and of
course managing if whatever is being fetched is in the particular cache
level.
+
+
+/*
+ * virResctrlMonitorGetStatistic
[...]
+str_id = strchr(++str_id, '_');
+if (!str_id)
+continue;
I think all of the above could be replaced by:
if (!(str_id = STRSKIP(ent->d_name, "mon_L")))
continue;
I personally am not a fan of pre-auto-incr. I get particularly
uncomfortable when it involves pointer arithmetic... But I think it's
unnecessary if you use STRSKIP
The interesting target directory name is like 'MON_L3_00' or 'MON_L3CODE_00'
if CDP is enabled. The last two numbers after second '_' character is the ID
number, now we need to locate the second '_' character.
One STRSKIP is not enough, still need a call of strchr to locate the second '_'
in
following way:
if (!(str_id = STRSKIP(ent->d_name, "mon_L")))
continue;
str_id = strchr(str_id , '_');
if (!str_id)
continue;
So MON_L3DATA_00 would do the same as would MON_L3FUTURE_00?
Yes. Here I am only interested in the last two digital numbers after
second character '_', which
is the node id.
Then let's STRSKIP(str_id, "_") - IOW: Skip to the next
Do you mean there steps in total?
if (!(str_id = STRSKIP(ent->d_name, "mon_L")))
continue;
str_id = strchr(str_id , '_');
if (!str_id)
continue;
if (!(str_id = STRSKIP(str_id, "_")))/* instead of STRSKIP(ent->d_name,
'_') */
continue;
if (virStrToLong_uip(str_id, NULL, 0, ) < 0)
goto cleanup;
The first STRSKIP is to get to the "level"#, right?
Yes. But I skipped the verify for cache level, we only support L3 cache
monitoring.
The second to the
"id"#. Maybe the variables should be named that way to make it clear
along with some comments.
Good suggestion.
I'll directly use 'node_id' for the name, and code would look like these:
/* Looking for directory that contains the resource utilization
*
Re: [libvirt] [PATCH v6 00/13] PCI passthrough support on s390
On 10/15/2018 09:30 AM, Boris Fiuczynski wrote: > On 10/14/18 2:47 PM, Andrea Bolognani wrote: >> On Fri, 2018-10-12 at 16:04 +0100, Daniel P. Berrangé wrote: >>> On Fri, Sep 28, 2018 at 04:46:13PM +0800, Yi Min Zhao wrote: >> [...] >>> function='0x0'> >>> >>> I'm not sure if this was discussed in earlier versions, but to me >>> this use of a child element looks wrong. >>> >>> What we're effectively saying is that s390 has a different addressing >>> scheme. It happens to share some fields with the current PCI addressing >>> scheme, but it is none the less a distinct scheme. >>> >>> IOW, I think it should be >>> >>> >> function='0x0' uid='0x0003' fid='0x0027'/> >>> >>> Of course internally we can still share much logic for assigning the >>> addreses between "pci" and "zpci". >> >> So what happens with PCI devices on s390 is that *two* devices will >> be added to the guest: one is the usual virtio-net-pci or what have >> you, which has its own PCI address allocated using the same algorithm >> as other architectures; the other one is a '-device zpci', which IIUC >> works basically like an adapter between the PCI device itself and the >> guest OS, and which is identified using uid and fid. >> >> Calling it a completely different address type seems like a bit of a >> stretch: there is definitely a PCI address involved, which is why the >> zPCI part was implemented through a potentially reusable "PCI address >> extension" mechanism. >> > I thought that we discussed this in v1 or v2 already when uid anf fid were > still embedded in the address element itself. In v5 Andrea suggested to model > the two zpci extension parameters outside as a child element of address which > corresponds kind of to what is happening in qemu (see Andreas paragraph > above). > The original idea was for users on s390 to make pci no different than on > other platforms. Creating a zpci address type would introduce the opposite. > Currently uid and fid are optional attributes for the user on s390. He can > simply enter any kind of pci address as for other platforms. If he does so on > s390 the uid and fid would be automatically generated for him. Only if he > chooses to specifically set these attributes himself he has to specify uid > and/or fid. Agreed. In QEMU this is really modelled as PCI (with the classic bus addresses). The fact that the guest uses the fid/uid instead does not make the PCI bus addresses in QEMU go away and it should not change the modelling of the devices. So I think that the current proposal from Andrea implemented by Yi Min and endorsed by Boris: is really the best solution. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 00/13] PCI passthrough support on s390
On 10/14/18 2:47 PM, Andrea Bolognani wrote: On Fri, 2018-10-12 at 16:04 +0100, Daniel P. Berrangé wrote: On Fri, Sep 28, 2018 at 04:46:13PM +0800, Yi Min Zhao wrote: [...] I'm not sure if this was discussed in earlier versions, but to me this use of a child element looks wrong. What we're effectively saying is that s390 has a different addressing scheme. It happens to share some fields with the current PCI addressing scheme, but it is none the less a distinct scheme. IOW, I think it should be Of course internally we can still share much logic for assigning the addreses between "pci" and "zpci". So what happens with PCI devices on s390 is that *two* devices will be added to the guest: one is the usual virtio-net-pci or what have you, which has its own PCI address allocated using the same algorithm as other architectures; the other one is a '-device zpci', which IIUC works basically like an adapter between the PCI device itself and the guest OS, and which is identified using uid and fid. Calling it a completely different address type seems like a bit of a stretch: there is definitely a PCI address involved, which is why the zPCI part was implemented through a potentially reusable "PCI address extension" mechanism. I thought that we discussed this in v1 or v2 already when uid anf fid were still embedded in the address element itself. In v5 Andrea suggested to model the two zpci extension parameters outside as a child element of address which corresponds kind of to what is happening in qemu (see Andreas paragraph above). The original idea was for users on s390 to make pci no different than on other platforms. Creating a zpci address type would introduce the opposite. Currently uid and fid are optional attributes for the user on s390. He can simply enter any kind of pci address as for other platforms. If he does so on s390 the uid and fid would be automatically generated for him. Only if he chooses to specifically set these attributes himself he has to specify uid and/or fid. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 08/19] util: Add interface for creating monitor group
On 10/12/2018 10:27 PM, John Ferlan wrote:
[...]
402 virResctrlMonitorDispose(void *obj)
403 {
404 virResctrlMonitorPtr monitor = obj;
405
406 virObjectUnref(monitor->alloc);
407 VIR_FREE(monitor->id);
408 VIR_FREE(monitor->path);
409 }
The one "thing" about the order of patches here is that it forces me to
look forward to ensure decisions made in previous patches will be
handled in the future.
Maybe combine virResctrlMonitorDispose and virResctrlMonitorCreate in one
patch? Will that make you look better for understanding how @monitor->alloc
is used.
That usually is best. I think the order has me off a bit too, but there
is no perfect solution for order. I find myself looking forward a lot
and trying to keep track.
I'll change the order of the series to fix this.
John
Thanks for review.
Huaqiang
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
On Mon, 2018-10-15 at 09:31 +0800, Yi Min Zhao wrote: > 在 2018/10/11 下午9:08, Andrea Bolognani 写道: > > Forgot to mention: it would be really nice if you added a negative > > test case showing that using zPCI addresses on eg. x86 will result > > in a validation error. > > OK. Let me have a try. Haven't added this kind of test before. It's very easy: just use DO_TEST_PARSE_ERROR() or DO_TEST_FAILURE() instead of DO_TEST() :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
