[libvirt] [PATCH] Fix nodeinfo output on PPC64 KVM hosts
The nodeinfo is reporting incorrect number of cpus and incorrect host topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only the primary thread in a core to be online, and the secondaries offlined. While scheduling a guest in, the kvm scheduler wakes up the secondaries to run in guest context. The host scheduling of the guests happen at the core level(as only primary thread is online). The kvm scheduler exploits as many threads of the core as needed by guest. Further, starting POWER8, the processor allows splitting a physical core into multiple subcores with 2 or 4 threads each. Again, only the primary thread in a subcore is online in the host. The KVM-PPC scheduler allows guests to exploit all the offline threads in the subcore, by bringing them online when needed. (Kernel patches on split-core http://www.spinics.net/lists/kvm-ppc/msg09121.html) Recently with dynamic micro-threading changes in ppc-kvm, makes sure to utilize all the offline cpus across guests, and across guests with different cpu topologies. (https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html) Since the offline cpus are brought online in the guest context, it is safe to count them as online. Nodeinfo today discounts these offline cpus from cpu count/topology calclulation, and the nodeinfo output is not of any help and the host appears overcommited when it is actually not. The patch carefully counts those offline threads whose primary threads are online. The host topology displayed by the nodeinfo is also fixed when the host is in valid kvm state. Test results with and without fix: http://ur1.ca/mx25y Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com --- src/nodeinfo.c | 85 1 file changed, 85 insertions(+) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 2fafe2d..1a27080 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -32,6 +32,8 @@ #include sys/utsname.h #include sched.h #include conf/domain_conf.h +#include fcntl.h +#include sys/ioctl.h #if defined(__FreeBSD__) || defined(__APPLE__) # include sys/time.h @@ -428,6 +430,10 @@ virNodeParseNode(const char *node, unsigned int cpu; int online; int direrr; +int kvmfd = -1; +int lastonline; +int threads_per_subcore = 1; +bool valid_ppc64_kvmhost_mode = false; *threads = 0; *cores = 0; @@ -464,6 +470,66 @@ virNodeParseNode(const char *node, sock_max++; +if (ARCH_IS_PPC64(arch)) { + /* PPC-KVM needs the secondary threads of a core to be offline on the +* host. The kvm schedular brings the secondary threads online in the +* guest context. Moreover, P8 processor has split-core capability +* where, there can be 1,2 or 4 subcores per core. The primaries of the +* subcores alone will be online on the host for a subcore in the +* host. Even though the actual threads per core for P8 processor is 8, +* depending on the subcores_per_core = 1, 2 or 4, the threads per +* subcore will vary accordingly to 8, 4 and 2 repectively. +* So, On host threads_per_core what is arrived at from sysfs in the +* current logic is actually the subcores_per_core. Threads per subcore +* can only be obtained from the kvm device. For example, on P8 wih 1 +* core having 8 threads, sub_cores_percore=4, the threads 0,2,4 6 +* will be online. The sysfs reflects this and in the current logic +* variable 'threads' will be 4 which is nothing but subcores_per_core. +* If the user tampers the cpu online/offline states using chcpu or other +* means, then it is an unsupported configuration for kvm. +* The code below tries to keep in mind +* - when the libvirtd is run inside a KVM guest or Phyp based guest. +* - Or on the kvm host where user manually tampers the cpu states to +*offline/online randomly. +*/ +# if HAVE_LINUX_KVM_H +# include linux/kvm.h +kvmfd = open(/dev/kvm, O_RDONLY); +if (kvmfd = 0) { +# ifdef KVM_CAP_PPC_SMT +valid_ppc64_kvmhost_mode = true; +/* For Phyp and KVM based guests the ioctl for KVM_CAP_PPC_SMT + * returns zero and both primary and secondary threads will be + * online. Dont try to alter or interpret anything here. + */ +threads_per_subcore = ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_SMT); +if (threads_per_subcore == 0) +valid_ppc64_kvmhost_mode = false; +# endif +} +VIR_FORCE_CLOSE(kvmfd); +# endif +rewinddir(cpudir); +lastonline = -1; +while (valid_ppc64_kvmhost_mode + ((direrr = virDirRead(cpudir, cpudirent, node)) 0)) { +if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1) +continue; +if ((online = virNodeGetCpuValue(node, cpu, online, 1)) 0) +
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
On Wed, Jun 24, 2015 at 06:17:35PM +0200, Johannes Holmberg wrote: - On 24 Jun, 2015, at 09:14, Martin Kletzander mklet...@redhat.com wrote: Why don't you just set XMLFILE=$TMPFILE ? That would get rid of lot of the code changes below and would be more readable. Other than that it looks good. Good question, with a somewhat long answer. I initially did exactly what you suggest, and yes, it makes for simpler code. The only problem with it is that when xmllint finds a problem with the xml, it will output a string on the format file name:line number: error description. If the file was provided on stdin, the file name will be something like /tmp/virt-xml.asdf which will not make much sense to the user. So instead I tried to be consistent: if virt-xml-validate received a filename, xmllint will receive a filename. If virt-xml-validate received data on stdin, xmllint will receive data on stdin. This way, when the xml is provided on stdin and xmllint finds a problem, the file name in the error message will just show up as -, which is probably more in line with what the user was expecting. Anyway, it's not something I feel super-strongly about, so if you prefer to have it changed, just let me know and I'll have an updated patch for you. I'm on the edge here. You've got perfectly good point here. If there was at least a way of simplifying each condition somehow. Each such approach pollutes the code with 'eval' or new function that makes it even more unreadable :( Anyway, I'm going into too much unnecessary details. Thanks to your explanation I'm OK with your approach, I would suggest just one teeny tiny change, and that's using 'xmllint - $file' instead of 'cat $file | xmllint -'. If that's ok with you, just Cc me on the v2 and I'll have a look at it. Thanks, Martin signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Added hook for network modification event
On Tue, Jun 23, 2015 at 09:01:58AM +0200, Anton Khramov wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1181539 Signed-off-by: Anton Khramov webmas...@ulzone.ru --- src/network/bridge_driver.c | 5 + src/util/virhook.c | 3 ++- src/util/virhook.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3d6721b..f5fc6f4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3307,6 +3307,11 @@ networkUpdate(virNetworkPtr net, if (needFirewallRefresh networkAddFirewallRules(network-def) 0) goto cleanup; +if (needFirewallRefresh +networkRunHook(network, NULL, NULL, VIR_HOOK_NETWORK_OP_UPDATED, + VIR_HOOK_SUBOP_BEGIN) 0) +goto cleanup; + Any reason why the hook should be called only if the firewall is being refreshed? if (flags VIR_NETWORK_UPDATE_AFFECT_CONFIG) { /* save updated persistent config to disk */ if (virNetworkSaveConfig(driver-networkConfigDir, diff --git a/src/util/virhook.c b/src/util/virhook.c index ee19382..516e4f4 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -93,7 +93,8 @@ VIR_ENUM_IMPL(virHookNetworkOp, VIR_HOOK_NETWORK_OP_LAST, started, stopped, plugged, - unplugged) + unplugged, + updated) static int virHooksFound = -1; diff --git a/src/util/virhook.h b/src/util/virhook.h index 550ef84..021308d 100644 --- a/src/util/virhook.h +++ b/src/util/virhook.h @@ -82,6 +82,7 @@ typedef enum { VIR_HOOK_NETWORK_OP_STOPPED,/* network has stopped */ VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, /* an interface has been plugged into the network */ VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED,/* an interface was unplugged from the network */ +VIR_HOOK_NETWORK_OP_UPDATED,/* network has been modified */ While it's enough for the code to work, you also need to update the documentation (docs/hooks.html.in in particular). VIR_HOOK_NETWORK_OP_LAST, } virHookNetworkOpType; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Plans for next release
[adding Dan to Cc as he asked me about this right before I had to leave for an appointment] On Thu, Jun 25, 2015 at 10:08:10PM +0800, Daniel Veillard wrote: Sorry I nearly forgot, but we should enter freeze soon to try to hit the end of the month target. I see that Martin's APIs for admin support has been merged in so that would be libvirt-1.3.0. Sorry from me too, but I planned to merge the admin API earlier so we can finish the design of first APIs. Unfortunately, everything got prolonged and we're right before the freeze with only Open and Close APIs. And because I really dislike when we create APIs that return unsupported just so we have them in, I'd be in favour for rather adding one useful API instead of 5 non-working ones. The other option is to conditionally disable the code for Admin API just for the release and bump the minor version for the next release when we have some useful stuff in. Plan is to enter freeze this w.e. and try to release by the end of the week, I hope this works for everyone, thanks, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
On Thu, Jun 25, 2015 at 11:46:41AM +0200, Ján Tomko wrote: On Thu, Jun 25, 2015 at 08:44:17AM +0200, Martin Kletzander wrote: On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote: At this point you can see that it all comes down to what name we want to give the subelement; within that, we give the exact name of the qemu device, and the exact name/value of any qemu options that we set to non-default values. Somebody *please* have an opinion on the name for this, because none of these strikes me as better or worse (well, I think I dislike driver To be honest, I kinds dislike all of them. Not that they would be chosen poorly, no, it's simply because the good sensible choice is unavailable due to another poor decision in the past (this may be another point for Michal's talk on KVM Forum). Thinking about it I must say I don't like how target (which is supposed to match a place where the device appears for the guest) is used for the model specification, on the other hand (ab)using 'model' element for the specification of an address in guest (that's what I understand chassis and port are) doesn't feel any better. What if we go with two of those elements? Would that be too much pain? E.g.: controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' model type='ioh3420'/ target chassis='3' port='0x21'/ /controller I like this, essentially a subModel without the camelCase. One small nit: model name='ioh4320'/ would look nicer to me, but we're using model type=/ in at least one other place, so I'm fine with both. I didn't think about the 'type', I probably just typed it in there because each line had a 'type' already in :) I like 'name' better too, but diving deeply into it, I have no strong opinions on that. Jan I understand this might look like an overkill, but I think it's better safe then sorry, I guess I just see us not so far in the future regretting any decision made now. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] driver level connection close event
Notify of connection close event from parallels driver (possibly) wrapped in the remote driver. Changes from v1: 1. fix comment style issues 2. remove spurious whitespaces 3. move rpc related part from vz patch to second(rpc) patch 4. remove unnecessary locks for immutable closeCallback in first patch. Discussion. In 1 and 2 patch we forced to some decisions because we don't have a weak reference mechanics. 1 patch. --- virConnectCloseCallback is introduced because we can not reference the connection object itself when setting a network layer callback because of how connection close works. A connection close procedure is next: 1. client closes connection 2. a this point nobody else referencing a connection and it is disposed 3. connection dispose unreferencing network connection 4. network connection disposes Thus if we referece a connection in network close callback we never get step 2. virConnectCloseCallback broke this cycle but at cost that clients MUST unregister explicitly before closing connection. This is not good as this unregistration is not really neaded. Client is not telling that it does not want to receive events anymore but rather forced to obey some implementation-driven rules. 2 patch. --- We impose requirements on driver implementations which is fragile. Moreover we again need to make explicit unregistrations. Implementation of domain events illustrates this point. remoteDispatchConnectDomainEventRegister does not reference NetClient and makes unregistration before NetClient is disposed but drivers do not meet the formulated requirements. Object event system release lock before delivering event for re-entrance purposes. Shortly we have 2 undesired consequences here. 1. Mandatory unregistration. 2. Imposing multi-threading requirements. Introduction of weak pointers could free us from these artifacts. Next weak reference workflow illustrates this. 1. Take weak reference on object of interest before passing to party. This doesn't break disposing mechanics as weak eference does not prevent from disposing object. Object is disposed but memory is not freed yet if there are weak references. 2. When callback is called we are safe to check if pointer dangling as we make a weak reference before. 3. Release weak reference and this trigger memory freeing if there are no more weak references. daemon/libvirtd.h|1 + daemon/remote.c | 86 +++ src/datatypes.c | 115 +++-- src/datatypes.h | 21 ++-- src/driver-hypervisor.h | 12 src/libvirt-host.c | 77 +--- src/remote/remote_driver.c | 106 +- src/remote/remote_protocol.x | 24 - src/remote_protocol-structs |6 ++ src/vz/vz_driver.c | 26 + src/vz/vz_sdk.c | 29 +++ src/vz/vz_utils.h|3 + -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] daemon: relay connection close event related functions
From: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com Add conneciton close subscription/unsubscription and event rpc. Now remote driver firing connection close event in 2 cases. 1. connection to daemon closed, as previously 2. as a relay of connection close event from wrapped driver As it commented out in remoteDispatchConnectCloseCallbackRegister we impose some multi-thread requirements on drivers implementations. This is the same approach as in for example remoteDispatchConnectDomainEventRegister. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- daemon/libvirtd.h|1 + daemon/remote.c | 86 ++ src/remote/remote_driver.c | 53 +- src/remote/remote_protocol.x | 24 +++- src/remote_protocol-structs |6 +++ 5 files changed, 167 insertions(+), 3 deletions(-) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 8c1a904..5b2d2ca 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -60,6 +60,7 @@ struct daemonClientPrivate { size_t nnetworkEventCallbacks; daemonClientEventCallbackPtr *qemuEventCallbacks; size_t nqemuEventCallbacks; +bool closeRegistered; # if WITH_SASL virNetSASLSessionPtr sasl; diff --git a/daemon/remote.c b/daemon/remote.c index e9e2dca..366ecd5 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1189,6 +1189,20 @@ remoteRelayDomainQemuMonitorEvent(virConnectPtr conn, VIR_FREE(details_p); } +static +void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int reason, void *opaque) +{ +virNetServerClientPtr client = opaque; + +VIR_DEBUG(Relaying connection closed event, reason %d, reason); + +remote_connect_event_connection_closed_msg msg = { reason }; +remoteDispatchObjectEventSend(client, remoteProgram, + REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED, + (xdrproc_t)xdr_remote_connect_event_connection_closed_msg, + msg); +} + /* * You must hold lock for at least the client * We don't free stuff here, merely disconnect the client's @@ -1251,6 +1265,12 @@ void remoteClientFreeFunc(void *data) } VIR_FREE(priv-qemuEventCallbacks); +if (priv-closeRegistered) { +if (virConnectUnregisterCloseCallback(priv-conn, + remoteRelayConnectionClosedEvent) 0) +VIR_WARN(unexpected close callback event deregister failure); +} + virConnectClose(priv-conn); virIdentitySetCurrent(NULL); @@ -3489,6 +3509,72 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server ATTRIBUTE_UNUSED, return rv; } +static int +remoteDispatchConnectCloseCallbackRegister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr) +{ +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +virMutexLock(priv-lock); + +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +/* NetClient is passed to driver but not referenced. + This imposes next requirements on drivers implementation. + Driver must serialize unregistering and event delivering operations. + Thus as we unregister callback before unreferencing NetClient + remoteRelayConnectionClosedEvent is safe to use NetClient. */ +if (virConnectRegisterCloseCallback(priv-conn, remoteRelayConnectionClosedEvent, client, NULL) 0) +goto cleanup; + +priv-closeRegistered = true; + +rv = 0; + + cleanup: +virMutexUnlock(priv-lock); +if (rv 0) +virNetMessageSaveError(rerr); +return rv; +} + +static int +remoteDispatchConnectCloseCallbackUnregister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr) +{ +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +virMutexLock(priv-lock); + +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (virConnectUnregisterCloseCallback(priv-conn, remoteRelayConnectionClosedEvent) 0) +goto cleanup; + +priv-closeRegistered = false; + +rv = 0; + + cleanup: +virMutexUnlock(priv-lock); +if (rv 0) +virNetMessageSaveError(rerr); +return rv; +} /*** * Register /
[libvirt] [PATCH v2 3/3] vz: implement connection close notification
From: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com Reuse virConnectCloseCallback to implement connection close event functions. This way we automatically meet multi-thread requirements on unregistering/notification. --- src/vz/vz_driver.c | 26 ++ src/vz/vz_sdk.c| 29 + src/vz/vz_utils.h |3 +++ 3 files changed, 58 insertions(+), 0 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index d9ddd4f..e3d0fdc 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -268,6 +268,9 @@ vzOpenDefault(virConnectPtr conn) if (prlsdkSubscribeToPCSEvents(privconn)) goto error; +if (!(privconn-closeCallback = virGetConnectCloseCallback())) +goto error; + conn-privateData = privconn; if (prlsdkLoadDomains(privconn)) @@ -276,6 +279,8 @@ vzOpenDefault(virConnectPtr conn) return VIR_DRV_OPEN_SUCCESS; error: +virObjectUnref(privconn-closeCallback); +privconn-closeCallback = NULL; virObjectUnref(privconn-domains); virObjectUnref(privconn-caps); virStoragePoolObjListFree(privconn-pools); @@ -350,6 +355,8 @@ vzConnectClose(virConnectPtr conn) virObjectUnref(privconn-caps); virObjectUnref(privconn-xmlopt); virObjectUnref(privconn-domains); +virObjectUnref(privconn-closeCallback); +privconn-closeCallback = NULL; virObjectEventStateFree(privconn-domainEventState); prlsdkDisconnect(privconn); conn-privateData = NULL; @@ -1337,6 +1344,23 @@ vzDomainBlockStatsFlags(virDomainPtr domain, return ret; } +static int +vzConnectRegisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ +vzConnPtr privconn = conn-privateData; +return virConnectCloseCallbackRegister(privconn-closeCallback, conn, cb, opaque, freecb); +} + +static int +vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb ATTRIBUTE_UNUSED) +{ +vzConnPtr privconn = conn-privateData; +virConnectCloseCallbackUnregister(privconn-closeCallback); +return 0; +} static virHypervisorDriver vzDriver = { .name = vz, @@ -1389,6 +1413,8 @@ static virHypervisorDriver vzDriver = { .domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */ .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */ .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */ +.connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.0 */ +.connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.0 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 388ea19..5f38709 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1745,6 +1745,32 @@ prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) return; } +static +void prlsdkHandleDispatcherConnectionClosed(vzConnPtr privconn) +{ +virConnectCloseCallbackCall(privconn-closeCallback, VIR_CONNECT_CLOSE_REASON_EOF); +} + +static void +prlsdkHandleDispatcherEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) +{ +PRL_RESULT pret = PRL_ERR_FAILURE; +PRL_EVENT_TYPE prlEventType; + +pret = PrlEvent_GetType(prlEvent, prlEventType); +prlsdkCheckRetGoto(pret, error); + +switch (prlEventType) { +case PET_DSP_EVT_DISP_CONNECTION_CLOSED: +prlsdkHandleDispatcherConnectionClosed(privconn); +break; +default: +VIR_DEBUG(Skipping dispatcher event of type %d, prlEventType); +} + error: +return; +} + static PRL_RESULT prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) { @@ -1772,6 +1798,9 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) // above function takes own of event prlEvent = PRL_INVALID_HANDLE; break; +case PIE_DISPATCHER: +prlsdkHandleDispatcherEvent(privconn, prlEvent); +break; default: VIR_DEBUG(Skipping event of issuer type %d, prlIssuerType); } diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 9b46bf9..b0dc3d8 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -32,6 +32,7 @@ # include conf/network_conf.h # include virthread.h # include virjson.h +# include datatypes.h # define vzParseError() \ virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \ @@ -69,6 +70,8 @@ struct _vzConn { virObjectEventStatePtr domainEventState; virStorageDriverStatePtr storageState; const char *drivername; +/* Immutable pointer, self-locking APIs */ +virConnectCloseCallbackPtr closeCallback; }; typedef struct _vzConn vzConn; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] remote: move connection close callback to driver level
From: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com 1. Introduce connect(Un)RegisterCloseCallback driver functions. 2. virConnect(Un)RegisterCloseCallback now works through driver. 3. virConnectCloseCallback is factored from virConnect but mostly stay the same. Notice however that virConnect object is not referenced in virConnectCloseCallback anymore. It is safe. Explanation. Previous version of callback object keeps reference to connection. This leads to undocumented rule that all clients must exlicitly unregister close callback before closing connection or connection will never be disposed. As callback unregistering and close event delivering are serialized thru callback object lock and unregistering zeros connection object we will never get dangling pointer on delivering. 4. callback object doesn't check callback on unregistering. The reason is that it will helps us write registering/unregistering with atomic behaviour for remote driver as it can be seen in next patch. Moreover it is not really meaningful to check callback on unregistering. 5. virNetClientSetCloseCallback call is removed from doRemoteClose as it is excessive for the same reasons as in point 3. Unregistering MUST be called and this prevents from firing event on close initiated by client. I'm not sure where callback object should be so it stays in datatype.c Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/datatypes.c| 115 +--- src/datatypes.h| 21 ++-- src/driver-hypervisor.h| 12 + src/libvirt-host.c | 77 ++ src/remote/remote_driver.c | 57 -- 5 files changed, 171 insertions(+), 111 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 12bcfc1..87a42cb 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -34,7 +34,7 @@ VIR_LOG_INIT(datatypes); virClassPtr virConnectClass; -virClassPtr virConnectCloseCallbackDataClass; +virClassPtr virConnectCloseCallbackClass; virClassPtr virDomainClass; virClassPtr virDomainSnapshotClass; virClassPtr virInterfaceClass; @@ -47,7 +47,7 @@ virClassPtr virStorageVolClass; virClassPtr virStoragePoolClass; static void virConnectDispose(void *obj); -static void virConnectCloseCallbackDataDispose(void *obj); +static void virConnectCloseCallbackDispose(void *obj); static void virDomainDispose(void *obj); static void virDomainSnapshotDispose(void *obj); static void virInterfaceDispose(void *obj); @@ -78,7 +78,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS_COMMON(basename, virClassForObjectLockable()) DECLARE_CLASS_LOCKABLE(virConnect); -DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData); +DECLARE_CLASS_LOCKABLE(virConnectCloseCallback); DECLARE_CLASS(virDomain); DECLARE_CLASS(virDomainSnapshot); DECLARE_CLASS(virInterface); @@ -119,14 +119,7 @@ virGetConnect(void) if (!(ret = virObjectLockableNew(virConnectClass))) return NULL; -if (!(ret-closeCallback = virObjectLockableNew(virConnectCloseCallbackDataClass))) -goto error; - return ret; - - error: -virObjectUnref(ret); -return NULL; } /** @@ -147,36 +140,102 @@ virConnectDispose(void *obj) virResetError(conn-err); virURIFree(conn-uri); +} -if (conn-closeCallback) { -virObjectLock(conn-closeCallback); -conn-closeCallback-callback = NULL; -virObjectUnlock(conn-closeCallback); +virConnectCloseCallbackPtr +virGetConnectCloseCallback(void) +{ +virConnectCloseCallbackPtr ret; -virObjectUnref(conn-closeCallback); -} +if (virDataTypesInitialize() 0) +return NULL; + +if (!(ret = virObjectLockableNew(virConnectCloseCallbackClass))) +return NULL; + +return ret; } +static void +virConnectCloseCallbackClean(virConnectCloseCallbackPtr obj) +{ +if (obj-freeCallback) +obj-freeCallback(obj-opaque); + +obj-callback = NULL; +obj-freeCallback = NULL; +obj-opaque = NULL; +obj-conn = NULL; +} -/** - * virConnectCloseCallbackDataDispose: - * @obj: the close callback data to release - * - * Release resources bound to the connection close callback. - */ static void -virConnectCloseCallbackDataDispose(void *obj) +virConnectCloseCallbackDispose(void *obj ATTRIBUTE_UNUSED) +{ +/* nothing really to do here */ +} + +int +virConnectCloseCallbackRegister(virConnectCloseCallbackPtr obj, +virConnectPtr conn, +virConnectCloseFunc cb, +void *opaque, +virFreeCallback freecb) { -virConnectCloseCallbackDataPtr cb = obj; +int ret = -1; -virObjectLock(cb); +virObjectLock(obj); -if (cb-freeCallback) -cb-freeCallback(cb-opaque); +if (obj-callback) { +/* Temporarily remove reporting to fix syntax-check. + Proper
Re: [libvirt] [sandbox PATCH 03/10] Add disk parameter to virt-sandbox
On Thu, Jun 25, 2015 at 03:49:04PM +0200, Cedric Bosdonnat wrote: On Thu, 2015-06-25 at 14:09 +0100, Daniel P. Berrange wrote: On Thu, Jun 25, 2015 at 01:27:23PM +0200, Eren Yagdiran wrote: Allow users to add disk images to their sandbox. Only disk images are supported so far, but the parameter is intentionally designed for future changes. --- bin/virt-sandbox.c | 37 + 1 file changed, 37 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) { GMainLoop *loop = NULL; GError *error = NULL; gchar *name = NULL; +gchar **disks = NULL; gchar **mounts = NULL; gchar **includes = NULL; gchar *includefile = NULL; @@ -92,6 +93,8 @@ int main(int argc, char **argv) { N_(name of the sandbox), NAME }, { root, 'r', 0, G_OPTION_ARG_STRING, root, N_(root directory of the sandbox), DIR }, +{ disk, ' ', 0, G_OPTION_ARG_STRING_ARRAY, disks, + N_(add a disk in the guest), TYPE:TARGET=SOURCE,FORMAT=FORMAT }, { mount, 'm', 0, G_OPTION_ARG_STRING_ARRAY, mounts, N_(mount a filesystem in the guest), TYPE:TARGET=SOURCE }, { include, 'i', 0, G_OPTION_ARG_STRING_ARRAY, includes, @@ -182,6 +185,13 @@ int main(int argc, char **argv) { gvir_sandbox_config_set_username(cfg, root); } +if (disks +!gvir_sandbox_config_add_disk_strv(cfg, disks, error)) { +g_printerr(_(Unable to parse disks: %s\n), + error error-message ? error-message : _(Unknown failure)); +goto cleanup; +} + if (mounts !gvir_sandbox_config_add_mount_strv(cfg, mounts, error)) { g_printerr(_(Unable to parse mounts: %s\n), @@ -319,6 +329,33 @@ inheriting the host's root filesystem. NB. CDIR must contain a matching install of the libvirt-sandbox package. This restriction may be lifted in a future version. +=item B--disk TYPE:TARGET=SOURCE,FORMAT=FORMAT + +Sets up a disk inside the sandbox by using BSOURCE with target device BTARGET +and type BTYPE and format BFORMAT. Example: file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 +Format is an optional parameter. + +=over 4 + +=item BTYPE + +Type parameter can be set to file. + +=item BTARGET + +Target parameter can be set to hda or any other libvirt supported target disk Per the discussion on the previous set of patches, we must *not* expose the libvirt disk target device string to the end user at all, because it means the user has to know about the differences in virtualizaton technology used. The latter patches in your series support the /dev/disk/by-tag/TAGNAME symlink to a /dev/TARGET device node, so the user should only be providing the TAGNAMNE and not the TARGET. ie, -disk file:data=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 would end up creating a link /dev/disk/by-tag/data which symlinked to /dev/vda on KVM, or /dev/sda on LXC, or /dev/xvda on Xen (hypothetical) etc. Ooops, I forgot that one when reviewing. The /dev/disk/by-tag/XXX is implemented, the man wasn't changed accordingly. I can change that before pushing if that is the only problem in the series. It isn't the only problem actually - the config API still names its methods 'get_disk_target' and when it creates the libvirt XML it is using this to set the disktarget dev=//disk attribute, so it goes deeper than just docs. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] vz: Reformat
25.06.2015 12:19, Maxim Nestratov пишет: 25.06.2015 11:18, Michal Privoznik пишет: There shouldn't be any functional change. Only some formatting nits fixed. Michal Privoznik (5): vz_driver: Reformat vz_network: Reformat vz_sdk: Reformat vz_storage: Reformat vz_utils: Reformat src/vz/vz_driver.c | 86 ++--- src/vz/vz_network.c | 26 +++ src/vz/vz_sdk.c | 209 ++-- src/vz/vz_storage.c | 58 +++ src/vz/vz_utils.h | 20 ++--- 5 files changed, 199 insertions(+), 200 deletions(-) ACK. Michal, thank you for this work. Maxim Nestratov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list By the way, Michal, how did you spot all these nits? Is there any tool to do it automatically? make syntax-check doesn't seem to be able to catch all this stuff. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 03/10] Add disk parameter to virt-sandbox
On Thu, Jun 25, 2015 at 01:27:23PM +0200, Eren Yagdiran wrote: Allow users to add disk images to their sandbox. Only disk images are supported so far, but the parameter is intentionally designed for future changes. --- bin/virt-sandbox.c | 37 + 1 file changed, 37 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) { GMainLoop *loop = NULL; GError *error = NULL; gchar *name = NULL; +gchar **disks = NULL; gchar **mounts = NULL; gchar **includes = NULL; gchar *includefile = NULL; @@ -92,6 +93,8 @@ int main(int argc, char **argv) { N_(name of the sandbox), NAME }, { root, 'r', 0, G_OPTION_ARG_STRING, root, N_(root directory of the sandbox), DIR }, +{ disk, ' ', 0, G_OPTION_ARG_STRING_ARRAY, disks, + N_(add a disk in the guest), TYPE:TARGET=SOURCE,FORMAT=FORMAT }, { mount, 'm', 0, G_OPTION_ARG_STRING_ARRAY, mounts, N_(mount a filesystem in the guest), TYPE:TARGET=SOURCE }, { include, 'i', 0, G_OPTION_ARG_STRING_ARRAY, includes, @@ -182,6 +185,13 @@ int main(int argc, char **argv) { gvir_sandbox_config_set_username(cfg, root); } +if (disks +!gvir_sandbox_config_add_disk_strv(cfg, disks, error)) { +g_printerr(_(Unable to parse disks: %s\n), + error error-message ? error-message : _(Unknown failure)); +goto cleanup; +} + if (mounts !gvir_sandbox_config_add_mount_strv(cfg, mounts, error)) { g_printerr(_(Unable to parse mounts: %s\n), @@ -319,6 +329,33 @@ inheriting the host's root filesystem. NB. CDIR must contain a matching install of the libvirt-sandbox package. This restriction may be lifted in a future version. +=item B--disk TYPE:TARGET=SOURCE,FORMAT=FORMAT + +Sets up a disk inside the sandbox by using BSOURCE with target device BTARGET +and type BTYPE and format BFORMAT. Example: file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 +Format is an optional parameter. + +=over 4 + +=item BTYPE + +Type parameter can be set to file. + +=item BTARGET + +Target parameter can be set to hda or any other libvirt supported target disk Per the discussion on the previous set of patches, we must *not* expose the libvirt disk target device string to the end user at all, because it means the user has to know about the differences in virtualizaton technology used. The latter patches in your series support the /dev/disk/by-tag/TAGNAME symlink to a /dev/TARGET device node, so the user should only be providing the TAGNAMNE and not the TARGET. ie, -disk file:data=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 would end up creating a link /dev/disk/by-tag/data which symlinked to /dev/vda on KVM, or /dev/sda on LXC, or /dev/xvda on Xen (hypothetical) etc. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: add RBD support to disk source pool translation
On 09/06/2015 17:33, John Ferlan wrote: I still think I need a tweak on what I posted as an update - the libvirtd restart path is always a bit tricky. The 'pooltype' isn't filled in at XML processing time and virStorageTranslateDiskSourcePool is only called after XML processing (sigh), so relying on it won't work. Working to figure out something generic and will repost my patch. John, I got stuck on this and would appreciate some advice to deal with the restart. With a revised patch it's still impossible to get libvirtd reconnecting to VMs using RBD source pool, because the network pool is never active so early. It goes like this : + qemuProcessReconnect ++ virStorageTranslateDiskSourcePool +++ virStoragePoolIsActive storagePoolIsActive = 0 ++ goto error ++ qemuProcessStop But seconds after it's seen as active. Isn't it a bad idea to consider a failure at translating source pool as an error in this code path? It looks quite dangerous whatever the pool protocol may be. After all the domain is still happily running anyway. Cheers -- Thibault -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Gathering CPUID data for x86 CPUs
Hi all, I'm working on improving our host CPU detection logic and I'd like to gather test data for various CPUs and include them in the libvirt's CPU unit test to make sure I don't introduce regressions. If you are willing to help me, please, send me the output of the following command: grep 'model name' /proc/cpuinfo | head -n1; cpuid -1r on various i686/x86_64 hosts you might own? Older or unusual CPUs are more than welcome. The cpuid tool can be usually found in a package called cpuid. If your distro does not provide such package and you are still willing to help, you can find the sources or binary packages at http://www.etallen.com/cpuid.html Private replies are preferred to reduce unnecessary mailing list traffic. Thanks a lot for your help, Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 08/10] Init-util : Common directory functions for init-common and init-qemu
Init-util provides common functions for creating directories for both Init-common and Init-qemu. Error handling needs to be done in calling function. --- libvirt-sandbox/Makefile.am | 4 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 57 +--- libvirt-sandbox/libvirt-sandbox-init-util.c | 58 + libvirt-sandbox/libvirt-sandbox-init-util.h | 41 4 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 8237c50..a7f1232 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -166,6 +166,7 @@ libvirt_sandbox_1_0_la_LDFLAGS = \ -version-info $(LIBVIRT_SANDBOX_VERSION_INFO) libvirt_sandbox_init_common_SOURCES = libvirt-sandbox-init-common.c \ + libvirt-sandbox-init-util.c \ $(SANDBOX_GENERATED_RPC_FILES) \ $(SANDBOX_RPC_FILES) \ $(NULL) @@ -216,7 +217,8 @@ libvirt_sandbox_init_lxc_LDADD = \ libvirt-sandbox-1.0.la \ $(NULL) -libvirt_sandbox_init_qemu_SOURCES = libvirt-sandbox-init-qemu.c +libvirt_sandbox_init_qemu_SOURCES = libvirt-sandbox-init-qemu.c \ + libvirt-sandbox-init-util.c libvirt_sandbox_init_qemu_CFLAGS = \ -DLIBEXECDIR=\$(libexecdir)\ \ -DSANDBOXCONFIGDIR=\$(sandboxconfigdir)\ \ diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 33cebed..aa1a092 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -43,6 +43,8 @@ #include sys/reboot.h #include termios.h +#include libvirt-sandbox-init-util.h + #define ATTR_UNUSED __attribute__((__unused__)) static void print_uptime (void); @@ -75,43 +77,13 @@ static void sig_child(int signum ATTR_UNUSED) } static void -mount_mkdir(const char *dir, int mode); - -static void -mount_mkparent(const char *dir, int mode) -{ -char *tmp = strrchr(dir, '/'); -if (tmp tmp != dir) { -char *parent = strndup(dir, tmp - dir); -mount_mkdir(parent, mode); -free(parent); -} -} - -static void -mount_mkdir(const char *dir, int mode) -{ -mount_mkparent(dir, 0755); - -if (debug) -fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s (mode=%o)\n, __func__, dir, mode); - -if (mkdir(dir, mode) 0) { -if (errno != EEXIST) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make directory %s: %s\n, -__func__, dir, strerror(errno)); -exit_poweroff(); -} -} -} - -static void mount_mkfile(const char *file, int mode) { int fd; -mount_mkparent(file, 0755); - +if (gvir_sandbox_init_util_mkparent(file, 0755, debug) 0) + exit_poweroff(); + if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s (mode=%o)\n, __func__, file, mode); @@ -132,8 +104,9 @@ mount_other_opts(const char *dst, const char *type, const char *opts, int mode) if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s (type=%s opts=%s)\n, __func__, dst, type, opts); -mount_mkdir(dst, mode); - +if (gvir_sandbox_init_util_mkdir(dst, mode,debug) 0) + exit_poweroff(); + if (mount(none, dst, type, 0, opts) 0) { fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot mount %s on %s (%s:%s): %s\n, __func__, type, dst, type, opts, strerror(errno)); @@ -160,8 +133,9 @@ mount_9pfs(const char *src, const char *dst, int mode, int readonly) if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s - %s (%d)\n, __func__, src, dst, readonly); -mount_mkdir(dst, mode); - +if (gvir_sandbox_init_util_mkdir(dst, mode, debug) 0) +exit_poweroff(); + if (readonly) flags |= MS_RDONLY; @@ -286,15 +260,18 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) __func__, source, strerror(errno)); exit_poweroff(); } -if (S_ISDIR(st.st_mode)) -mount_mkdir(target, 755); +if (S_ISDIR(st.st_mode)){ +if (gvir_sandbox_init_util_mkdir(target, 755, debug) 0) + exit_poweroff(); + } else mount_mkfile(target, 644); } else { if (strcmp(type, tmpfs) == 0) flags |= MS_NOSUID | MS_NODEV; -mount_mkdir(target, 0755); +if (gvir_sandbox_init_util_mkdir(target, 0755, debug) 0) +exit_poweroff(); } if (mount(source, target, type, flags,
[libvirt] [sandbox PATCH 03/10] Add disk parameter to virt-sandbox
Allow users to add disk images to their sandbox. Only disk images are supported so far, but the parameter is intentionally designed for future changes. --- bin/virt-sandbox.c | 37 + 1 file changed, 37 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) { GMainLoop *loop = NULL; GError *error = NULL; gchar *name = NULL; +gchar **disks = NULL; gchar **mounts = NULL; gchar **includes = NULL; gchar *includefile = NULL; @@ -92,6 +93,8 @@ int main(int argc, char **argv) { N_(name of the sandbox), NAME }, { root, 'r', 0, G_OPTION_ARG_STRING, root, N_(root directory of the sandbox), DIR }, +{ disk, ' ', 0, G_OPTION_ARG_STRING_ARRAY, disks, + N_(add a disk in the guest), TYPE:TARGET=SOURCE,FORMAT=FORMAT }, { mount, 'm', 0, G_OPTION_ARG_STRING_ARRAY, mounts, N_(mount a filesystem in the guest), TYPE:TARGET=SOURCE }, { include, 'i', 0, G_OPTION_ARG_STRING_ARRAY, includes, @@ -182,6 +185,13 @@ int main(int argc, char **argv) { gvir_sandbox_config_set_username(cfg, root); } +if (disks +!gvir_sandbox_config_add_disk_strv(cfg, disks, error)) { +g_printerr(_(Unable to parse disks: %s\n), + error error-message ? error-message : _(Unknown failure)); +goto cleanup; +} + if (mounts !gvir_sandbox_config_add_mount_strv(cfg, mounts, error)) { g_printerr(_(Unable to parse mounts: %s\n), @@ -319,6 +329,33 @@ inheriting the host's root filesystem. NB. CDIR must contain a matching install of the libvirt-sandbox package. This restriction may be lifted in a future version. +=item B--disk TYPE:TARGET=SOURCE,FORMAT=FORMAT + +Sets up a disk inside the sandbox by using BSOURCE with target device BTARGET +and type BTYPE and format BFORMAT. Example: file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 +Format is an optional parameter. + +=over 4 + +=item BTYPE + +Type parameter can be set to file. + +=item BTARGET + +Target parameter can be set to hda or any other libvirt supported target disk + +=item BSOURCE + +Source parameter needs to point a file which must be a one of the valid domain disk formats supported by qemu. + +=item BFORMAT + +Format parameter must be set to the same disk format as the file passed on source parameter. +This parameter is optional and the format can be guessed from the image extension + +=back + =item B-m TYPE:DST=SRC, B--mount TYPE:DST=SRC Sets up a mount inside the sandbox at BDST backed by BSRC. The -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 00/10] Patches for Libvirt-sandbox
Hello, These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. --Main diffs compared to previous patches. Since many hypervisors, including kvm, will not even honour requested names for disk devices we link each device under /dev/disk/by-tag e.g /dev/disk/by-tag/foobar - /dev/sda We populate disks.cfg with tag to device mapping when we build the sandbox. After that, in each init-process {Common,Qemu}, we basically read the configuration and populate the right symlinks under /dev/disk/by-tag The common functions for modifying directories are moved under Init-util. {Common,Qemu} inits are using them. Cédric Bosdonnat (2): Add gvir_sandbox_config_has_disks function qemu: use devtmpfs rather than tmpfs to auto-populate /dev Eren Yagdiran (8): Add an utility function for guessing filetype from file extension Add configuration object for disk support Add disk parameter to virt-sandbox Add disk support to the container builder Add disk support to machine builder Init-util : Common directory functions for init-common and init-qemu Common-init: Building symlink from disks.cfg Common-builder: /dev/disk/by-tag/thetag to /dev/vdN bin/virt-sandbox.c | 37 +++ libvirt-sandbox/Makefile.am| 7 +- .../libvirt-sandbox-builder-container.c| 37 ++- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 44 ++- libvirt-sandbox/libvirt-sandbox-builder.c | 73 - libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 +++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 ++ libvirt-sandbox/libvirt-sandbox-config.c | 300 + libvirt-sandbox/libvirt-sandbox-config.h | 11 + libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +++- libvirt-sandbox/libvirt-sandbox-init-qemu.c| 151 ++- libvirt-sandbox/libvirt-sandbox-init-util.c| 58 libvirt-sandbox/libvirt-sandbox-init-util.h| 41 +++ libvirt-sandbox/libvirt-sandbox-util.c | 72 + libvirt-sandbox/libvirt-sandbox-util.h | 5 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym| 5 + libvirt-sandbox/tests/test-config.c| 11 + po/POTFILES.in | 1 + 19 files changed, 1112 insertions(+), 149 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 10/10] Common-builder: /dev/disk/by-tag/thetag to /dev/vdN
Common builder counts the disks devices and populates disks.cfg according to that.Disk devices are always come first than host-based images.In builder-machine, mounts of the host-based images will be mounted later. --- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 6 +- libvirt-sandbox/libvirt-sandbox-builder.c | 73 +-- 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 4148d4b..db956cf 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -283,9 +283,10 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * error); gboolean ret = FALSE; GList *mounts = gvir_sandbox_config_get_mounts(config); +GList *disks = gvir_sandbox_config_get_disks(config); GList *tmp = NULL; size_t nHostBind = 0; -size_t nHostImage = 0; +guint nVirtioDev = g_list_length(disks); if (!fos) goto cleanup; @@ -304,7 +305,7 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * fstype = 9p; options = g_strdup(trans=virtio,version=9p2000.u); } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_HOST_IMAGE(mconfig)) { -source = g_strdup_printf(/dev/vd%c, (char)('a' + nHostImage++)); +source = g_strdup_printf(/dev/vd%c, (char)('a' + nVirtioDev++)); fstype = ext4; options = g_strdup(); } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_GUEST_BIND(mconfig)) { @@ -351,6 +352,7 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * cleanup: g_list_foreach(mounts, (GFunc)g_object_unref, NULL); g_list_free(mounts); +g_list_free(disks); if (fos) g_object_unref(fos); if (!ret) diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c index bcad652..d83fd9f 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder.c +++ b/libvirt-sandbox/libvirt-sandbox-builder.c @@ -312,14 +312,75 @@ static gboolean gvir_sandbox_builder_construct_features(GVirSandboxBuilder *buil return TRUE; } +static gboolean gvir_sandbox_builder_construct_disk_cfg(GVirSandboxConfig *config, +const gchar *statedir, +GError **error) +{ -static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *builder G_GNUC_UNUSED, - GVirSandboxConfig *config G_GNUC_UNUSED, - const gchar *statedir G_GNUC_UNUSED, - GVirConfigDomain *domain G_GNUC_UNUSED, - GError **error G_GNUC_UNUSED) +guint nVirtioDev = 0; +gchar *dskfile = g_strdup_printf(%s/config/disks.cfg, statedir); +GFile *file = g_file_new_for_path(dskfile); +GFileOutputStream *fos = g_file_replace(file, +NULL, +FALSE, +G_FILE_CREATE_REPLACE_DESTINATION, +NULL, +error); +gboolean ret = FALSE; +GList *disks = gvir_sandbox_config_get_disks(config); +GList *tmp = NULL; +const gchar *target; + +if (!fos) +goto cleanup; + +tmp = disks; +while (tmp) { +GVirSandboxConfigDisk *mconfig = GVIR_SANDBOX_CONFIG_DISK(tmp-data); +gchar *device = g_strdup_printf(/dev/vd%c, (char)('a' + (nVirtioDev)++)); +gchar *line; + +target = gvir_sandbox_config_disk_get_target(mconfig); + +line = g_strdup_printf(%s\t%s\n, + target, device); +g_free(device); + +if (!g_output_stream_write_all(G_OUTPUT_STREAM(fos), + line, strlen(line), + NULL, NULL, error)) { +g_free(line); +goto cleanup; +} +g_free(line); + +tmp = tmp-next; +} + +if (!g_output_stream_close(G_OUTPUT_STREAM(fos), NULL, error)) +goto cleanup; + +ret = TRUE; + cleanup: +g_list_foreach(disks, (GFunc)g_object_unref, NULL); +g_list_free(disks); +if (fos) +g_object_unref(fos); +if (!ret) +g_file_delete(file, NULL, NULL); +g_object_unref(file); +g_free(dskfile); +return ret; + +} + +static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *builder, + GVirSandboxConfig *config, +
[libvirt] [sandbox PATCH 05/10] Add disk support to machine builder
Use the new disk configuration in the container builder to provide disks in qemu sandboxes. The disks are virtio devices, but those shouldn't be known by the user. --- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 38 --- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 5e6bf72..4148d4b 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -174,7 +174,8 @@ static gchar *gvir_sandbox_builder_machine_mkinitrd(GVirSandboxConfig *config, if (gvir_sandbox_config_has_networks(config)) gvir_sandbox_config_initrd_add_module(initrd, virtio_net.ko); if (gvir_sandbox_config_has_mounts_with_type(config, - GVIR_SANDBOX_TYPE_CONFIG_MOUNT_HOST_IMAGE)) + GVIR_SANDBOX_TYPE_CONFIG_MOUNT_HOST_IMAGE) || +gvir_sandbox_config_has_disks(config)) gvir_sandbox_config_initrd_add_module(initrd, virtio_blk.ko); gvir_sandbox_config_initrd_add_module(initrd, virtio_console.ko); #if 0 @@ -503,9 +504,9 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde GVirConfigDomainConsole *con; GVirConfigDomainSerial *ser; GVirConfigDomainChardevSourcePty *src; -GList *tmp = NULL, *mounts = NULL, *networks = NULL; +GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL; size_t nHostBind = 0; -size_t nHostImage = 0; +size_t nVirtioDev = 0; gchar *configdir = g_strdup_printf(%s/config, statedir); gboolean ret = FALSE; @@ -538,6 +539,35 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde g_object_unref(fs); +tmp = disks = gvir_sandbox_config_get_disks(config); +while(tmp){ +GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp-data); + +if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){ +gchar *device = g_strdup_printf(vd%c, (char)('a' + nVirtioDev++)); + +disk = gvir_config_domain_disk_new(); +diskDriver = gvir_config_domain_disk_driver_new(); +gvir_config_domain_disk_set_type(disk, + gvir_sandbox_config_disk_get_disk_type(dconfig)); +gvir_config_domain_disk_driver_set_format(diskDriver, + gvir_sandbox_config_disk_get_format(dconfig)); +gvir_config_domain_disk_set_source(disk, gvir_sandbox_config_disk_get_source(dconfig)); +gvir_config_domain_disk_set_target_bus(disk, + GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO); +gvir_config_domain_disk_set_target_dev(disk, + device); +gvir_config_domain_disk_set_driver(disk, diskDriver); +gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(disk)); +g_object_unref(disk); +g_free(device); +} +tmp = tmp-next; +} + +g_list_foreach(disks, (GFunc)g_object_unref, NULL); +g_list_free(disks); + tmp = mounts = gvir_sandbox_config_get_mounts(config); while (tmp) { GVirSandboxConfigMount *mconfig = GVIR_SANDBOX_CONFIG_MOUNT(tmp-data); @@ -563,7 +593,7 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde GVirSandboxConfigMountFile *mfile = GVIR_SANDBOX_CONFIG_MOUNT_FILE(mconfig); GVirSandboxConfigMountHostImage *mimage = GVIR_SANDBOX_CONFIG_MOUNT_HOST_IMAGE(mconfig); GVirConfigDomainDiskFormat format; -gchar *target = g_strdup_printf(vd%c, (char)('a' + nHostImage++)); +gchar *target = g_strdup_printf(vd%c, (char)('a' + nVirtioDev++)); disk = gvir_config_domain_disk_new(); gvir_config_domain_disk_set_type(disk, GVIR_CONFIG_DOMAIN_DISK_FILE); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 04/10] Add disk support to the container builder
Use the new disk configuration in the container builder to provide disks in lxc containers sandboxes. --- .../libvirt-sandbox-builder-container.c| 37 +- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c index 59bfee1..3318c30 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c @@ -218,14 +218,49 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil GVirConfigDomainInterfaceNetwork *iface; GVirConfigDomainConsole *con; GVirConfigDomainChardevSourcePty *src; -GList *tmp = NULL, *mounts = NULL, *networks = NULL; +GVirConfigDomainDisk *disk; +GVirConfigDomainDiskDriver *diskDriver; +GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL; gchar *configdir = g_strdup_printf(%s/config, statedir); gboolean ret = FALSE; +size_t nVirtioDev = 0; if (!GVIR_SANDBOX_BUILDER_CLASS(gvir_sandbox_builder_container_parent_class)- construct_devices(builder, config, statedir, domain, error)) goto cleanup; + +tmp = disks = gvir_sandbox_config_get_disks(config); +while(tmp){ +GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp-data); + +if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){ + +gchar *device = g_strdup_printf(vd%c, (char)('a' + nVirtioDev++)); +disk = gvir_config_domain_disk_new(); +diskDriver = gvir_config_domain_disk_driver_new(); +gvir_config_domain_disk_set_type(disk, + gvir_sandbox_config_disk_get_disk_type(dconfig)); +gvir_config_domain_disk_driver_set_format(diskDriver, + gvir_sandbox_config_disk_get_format(dconfig)); +gvir_config_domain_disk_driver_set_name(diskDriver, nbd); +gvir_config_domain_disk_set_source(disk, + gvir_sandbox_config_disk_get_source(dconfig)); +gvir_config_domain_disk_set_target_bus(disk, + GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO); +gvir_config_domain_disk_set_target_dev(disk,device); +gvir_config_domain_disk_set_driver(disk, diskDriver); +gvir_config_domain_add_device(domain, + GVIR_CONFIG_DOMAIN_DEVICE(disk)); +g_object_unref(disk); +} +tmp = tmp-next; +} + +g_list_foreach(disks, (GFunc)g_object_unref, NULL); +g_list_free(disks); + + fs = gvir_config_domain_filesys_new(); gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_PASSTHROUGH); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 07/10] qemu: use devtmpfs rather than tmpfs to auto-populate /dev
From: Cédric Bosdonnat cbosdon...@suse.com When using devtmpfs we don't need to care about the device nodes creation: it's less risk to forget some. It also eases the creation of the devices in the init-qemu. --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 94 + 1 file changed, 1 insertion(+), 93 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 750c9de..33cebed 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -53,7 +53,6 @@ static int has_command_arg(const char *name, static int debug = 0; static char line[1024]; -static char line2[1024]; static void exit_poweroff(void) __attribute__((noreturn)); @@ -173,50 +172,6 @@ mount_9pfs(const char *src, const char *dst, int mode, int readonly) } } -static int virtioblk_major = 0; -static void -create_virtioblk_device(const char *dev) -{ -int minor; - -if (virtioblk_major == 0) { -FILE *fp = fopen(/proc/devices, r); -if (!fp) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot read /proc/devices: %s\n, -__func__, strerror(errno)); -exit_poweroff(); -} -while (fgets(line2, sizeof line2, fp)) { -if (strstr(line2, virtblk)) { -char *end; -long l = strtol(line2, end, 10); -if (line2 == end) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot extract device major from '%s'\n, -__func__, line2); -fclose(fp); -exit_poweroff(); -} -virtioblk_major = l; -break; -} -} -fclose(fp); - -if (virtioblk_major == 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot find virtioblk device major in /proc/devices\n, -__func__); -exit_poweroff(); -} -} - -minor = (dev[strlen(dev)-1] - 'a') * 16; - -if (mknod(dev, S_IFBLK |0700, makedev(virtioblk_major, minor)) 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make dev '%s' '%llu': %s\n, -__func__, dev, makedev(virtioblk_major, minor), strerror(errno)); -exit_poweroff(); -} -} int main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) @@ -283,59 +238,14 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) } /* Main special filesystems */ -mount_other(/dev, tmpfs, 0755); +mount_other(/dev, devtmpfs, 0755); mount_other_opts(/dev/pts, devpts, gid=5,mode=620,ptmxmode=000, 0755); mount_other(/sys, sysfs, 0755); mount_other(/proc, proc, 0755); //mount_other(/selinux, selinuxfs, 0755); mount_other(/dev/shm, tmpfs, 01777); -if (mkdir(/dev/input, 0777) 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make directory /dev/input: %s\n, -__func__, strerror(errno)); -exit_poweroff(); -} - -#define MKNOD(file, mode, dev) \ -do {\ -if (mknod(file, mode, dev) 0) { \ -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make dev %s %llu: %s\n, \ -__func__, file, (unsigned long long)dev, strerror(errno)); \ -exit_poweroff();\ -} \ -} while (0) - -umask(); -MKNOD(/dev/null, S_IFCHR |0666, makedev(1, 3)); -MKNOD(/dev/zero, S_IFCHR |0666, makedev(1, 5)); -MKNOD(/dev/full, S_IFCHR |0666, makedev(1, 7)); -MKNOD(/dev/random, S_IFCHR |0666, makedev(1, 8)); -MKNOD(/dev/urandom, S_IFCHR |0666, makedev(1, 9)); -MKNOD(/dev/console, S_IFCHR |0700, makedev(5, 1)); -MKNOD(/dev/tty, S_IFCHR |0700, makedev(5, 0)); -MKNOD(/dev/tty0, S_IFCHR |0700, makedev(4, 0)); -MKNOD(/dev/tty1, S_IFCHR |0700, makedev(4, 1)); -MKNOD(/dev/tty2, S_IFCHR |0700, makedev(4, 2)); -MKNOD(/dev/ttyS0, S_IFCHR |0700, makedev(4, 64)); -MKNOD(/dev/ttyS1, S_IFCHR |0700, makedev(4, 65)); -MKNOD(/dev/ttyS2, S_IFCHR |0700, makedev(4, 66)); -MKNOD(/dev/ttyS3, S_IFCHR |0700, makedev(4, 67)); -MKNOD(/dev/hvc0, S_IFCHR |0700, makedev(229, 0)); -MKNOD(/dev/hvc1, S_IFCHR |0700, makedev(229, 1)); -MKNOD(/dev/hvc2, S_IFCHR |0700, makedev(229, 2)); -MKNOD(/dev/fb, S_IFCHR |0700, makedev(29, 0)); -MKNOD(/dev/fb0, S_IFCHR |0700, makedev(29, 0)); -MKNOD(/dev/mem, S_IFCHR |0600, makedev(1, 1)); -MKNOD(/dev/rtc, S_IFCHR |0700, makedev(254, 0)); -MKNOD(/dev/rtc0, S_IFCHR |0700, makedev(254, 0)); -MKNOD(/dev/ptmx, S_IFCHR |0777, makedev(5, 2)); -MKNOD(/dev/input/event0, S_IFCHR |0700,
[libvirt] [sandbox PATCH 06/10] Add gvir_sandbox_config_has_disks function
From: Cédric Bosdonnat cbosdon...@suse.com Add helper function to check if a config contains disk devices. --- libvirt-sandbox/libvirt-sandbox-config.c | 7 +++ libvirt-sandbox/libvirt-sandbox-config.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 1 + 3 files changed, 9 insertions(+) diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index 3342706..f800cf9 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -1305,6 +1305,13 @@ gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, } +gboolean gvir_sandbox_config_has_disks(GVirSandboxConfig *config) +{ +GVirSandboxConfigPrivate *priv = config-priv; +return priv-disks != NULL; +} + + /** * gvir_sandbox_config_add_mount: * @config: (transfer none): the sandbox config diff --git a/libvirt-sandbox/libvirt-sandbox-config.h b/libvirt-sandbox/libvirt-sandbox-config.h index deaea68..ebbebf2 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.h +++ b/libvirt-sandbox/libvirt-sandbox-config.h @@ -131,6 +131,7 @@ gboolean gvir_sandbox_config_add_disk_strv(GVirSandboxConfig *config, gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, const char *disk, GError **error); +gboolean gvir_sandbox_config_has_disks(GVirSandboxConfig *config); void gvir_sandbox_config_add_mount(GVirSandboxConfig *config, GVirSandboxConfigMount *mnt); diff --git a/libvirt-sandbox/libvirt-sandbox.sym b/libvirt-sandbox/libvirt-sandbox.sym index bb717ed..e5f8660 100644 --- a/libvirt-sandbox/libvirt-sandbox.sym +++ b/libvirt-sandbox/libvirt-sandbox.sym @@ -217,4 +217,5 @@ LIBVIRT_SANDBOX_0.5.2 { gvir_sandbox_config_add_disk_strv; gvir_sandbox_config_add_disk_opts; gvir_sandbox_config_disk_get_type; +gvir_sandbox_config_has_disks; } LIBVIRT_SANDBOX_0.2.1; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 00/10] Patches for Libvirt-sandbox
Hello, These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. --Main diffs compared to previous patches. Since many hypervisors, including kvm, will not even honour requested names for disk devices we link each device under /dev/disk/by-tag e.g /dev/disk/by-tag/foobar - /dev/sda We populate disks.cfg with tag to device mapping when we build the sandbox. After that, in each init-process {Common,Qemu}, we basically read the configuration and populate the right symlinks under /dev/disk/by-tag The common functions for modifying directories are moved under Init-util. {Common,Qemu} inits are using them. Cédric Bosdonnat (2): Add gvir_sandbox_config_has_disks function qemu: use devtmpfs rather than tmpfs to auto-populate /dev Eren Yagdiran (8): Add an utility function for guessing filetype from file extension Add configuration object for disk support Add disk parameter to virt-sandbox Add disk support to the container builder Add disk support to machine builder Init-util : Common directory functions for init-common and init-qemu Common-init: Building symlink from disks.cfg Common-builder: /dev/disk/by-tag/thetag to /dev/vdN bin/virt-sandbox.c | 37 +++ libvirt-sandbox/Makefile.am| 7 +- .../libvirt-sandbox-builder-container.c| 37 ++- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 44 ++- libvirt-sandbox/libvirt-sandbox-builder.c | 73 - libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 +++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 ++ libvirt-sandbox/libvirt-sandbox-config.c | 300 + libvirt-sandbox/libvirt-sandbox-config.h | 11 + libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +++- libvirt-sandbox/libvirt-sandbox-init-qemu.c| 151 ++- libvirt-sandbox/libvirt-sandbox-init-util.c| 58 libvirt-sandbox/libvirt-sandbox-init-util.h| 41 +++ libvirt-sandbox/libvirt-sandbox-util.c | 72 + libvirt-sandbox/libvirt-sandbox-util.h | 5 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym| 5 + libvirt-sandbox/tests/test-config.c| 11 + po/POTFILES.in | 1 + 19 files changed, 1112 insertions(+), 149 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 09/10] Common-init: Building symlink from disks.cfg
Similar to the existing mounts.cfg, the mapping between the device and the tag is passed by a new disks.cfg file. Common-init reads disks.cfg and maps the tags to corresponding devices --- libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c b/libvirt-sandbox/libvirt-sandbox-init-common.c index 68f96ba..f8b2ea5 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-common.c +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c @@ -46,6 +46,7 @@ #include grp.h #include libvirt-sandbox-rpcpacket.h +#include libvirt-sandbox-init-util.h static gboolean debug = FALSE; static gboolean verbose = FALSE; @@ -60,6 +61,51 @@ static void sig_child(int sig ATTR_UNUSED) abort(); } +static gboolean setup_disk_tags(void) { +FILE *fp; +gboolean ret = FALSE; +static char line[1024]; +if (debug) +fprintf(stderr, libvirt-sandbox-init-common: %s: populate /dev/disk/by-tag/\n, + __func__); +fp = fopen(SANDBOXCONFIGDIR /disks.cfg, r); +if (fp == NULL) { +fprintf(stderr, libvirt-sandbox-init-common: %s: cannot open SANDBOXCONFIGDIR /disks.cfg: %s\n, +__func__, strerror(errno)); + +goto cleanup; +} +gvir_sandbox_init_util_mkdir(/dev/disk/by-tag, 0755, debug == TRUE ? 1 : 0); +while (fgets(line, sizeof line, fp)) { +char path[1024]; +char *tag = line; +char *device = strchr(tag, '\t'); +*device = '\0'; +device++; +char *tmp = strchr(device, '\n'); +*tmp = '\0'; + +if (sprintf(path, /dev/disk/by-tag/%s, tag) 0) { +fprintf(stderr, libvirt-sandbox-init-common: %s: cannot compute disk path: %s\n, +__func__, strerror(errno)); +goto cleanup; +} + +if (debug) +fprintf(stderr, libvirt-sandbox-init-common: %s: %s - %s\n, +__func__, path, device); + +if (symlink(device, path) 0) { +fprintf(stderr, libvirt-sandbox-init-common: %s: cannot create symlink %s - %s: %s\n, +__func__, path, device, strerror(errno)); +goto cleanup; +} +} +ret = TRUE; + cleanup: +fclose(fp); +return ret; +} static int start_shell(void) @@ -258,8 +304,6 @@ static gboolean setup_network_device(GVirSandboxConfigNetwork *config, return ret; } - - static gboolean setup_network(GVirSandboxConfig *config, GError **error) { int i = 0; @@ -1215,6 +1259,9 @@ int main(int argc, char **argv) { start_shell() 0) exit(EXIT_FAILURE); +if (!setup_disk_tags()) +exit(EXIT_FAILURE); + if (!setup_network(config, error)) goto error; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 01/10] Add an utility function for guessing filetype from file extension
Consider the file name extension as the image type, except for .img that are usually RAW images --- libvirt-sandbox/Makefile.am| 1 + libvirt-sandbox/libvirt-sandbox-util.c | 72 ++ libvirt-sandbox/libvirt-sandbox-util.h | 5 +++ po/POTFILES.in | 1 + 4 files changed, 79 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 96302cb..6917f04 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -84,6 +84,7 @@ SANDBOX_HEADER_FILES = \ $(NULL) SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ + libvirt-sandbox-util.c \ libvirt-sandbox-config.c \ libvirt-sandbox-config-network.c \ libvirt-sandbox-config-network-address.c \ diff --git a/libvirt-sandbox/libvirt-sandbox-util.c b/libvirt-sandbox/libvirt-sandbox-util.c new file mode 100644 index 000..32d4c4e --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-util.c @@ -0,0 +1,72 @@ +/* + * libvirt-sandbox-util.c: libvirt sandbox util functions + * + * Copyright (C) 2015 Universitat Polit??cnica de Catalunya. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran erenyagdi...@gmail.com + */ + +#include config.h +#include string.h +#include errno.h +#include glib/gi18n.h + +#include libvirt-sandbox/libvirt-sandbox.h + +#define GVIR_SANDBOX_UTIL_ERROR gvir_sandbox_util_error_quark() + +static GQuark +gvir_sandbox_util_error_quark(void) +{ +return g_quark_from_static_string(gvir-sandbox-util); +} + +gint gvir_sandbox_util_guess_image_format(const gchar *path, + GError **error) +{ +gchar *tmp; + +if ((tmp = strchr(path, '.')) == NULL) { +return -1; +} +tmp = tmp + 1; + +if (strcmp(tmp,img)==0){ + return GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW; +} + +return gvir_sandbox_util_disk_format_from_str(tmp, error); +} + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value, +GError **error) +{ +GEnumClass *enum_class = g_type_class_ref(GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT); +GEnumValue *enum_value; +gint ret = -1; + +if (!(enum_value = g_enum_get_value_by_nick(enum_class, value))) { +g_set_error(error, GVIR_SANDBOX_UTIL_ERROR, 0, +_(Unknown disk format '%s'), value); +goto cleanup; +} +ret = enum_value-value; + + cleanup: +g_type_class_unref(enum_class); +return ret; +} diff --git a/libvirt-sandbox/libvirt-sandbox-util.h b/libvirt-sandbox/libvirt-sandbox-util.h index ae6b74b..890ae7f 100644 --- a/libvirt-sandbox/libvirt-sandbox-util.h +++ b/libvirt-sandbox/libvirt-sandbox-util.h @@ -29,6 +29,11 @@ G_BEGIN_DECLS +gint gvir_sandbox_util_guess_image_format(const gchar *path, GError **error); + + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value, GError **error); + /** * LIBVIRT_SANDBOX_CLASS_PADDING: (skip) */ diff --git a/po/POTFILES.in b/po/POTFILES.in index 653abc5..f291b98 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -9,3 +9,4 @@ libvirt-sandbox/libvirt-sandbox-console-rpc.c libvirt-sandbox/libvirt-sandbox-context.c libvirt-sandbox/libvirt-sandbox-init-common.c libvirt-sandbox/libvirt-sandbox-rpcpacket.c +libvirt-sandbox/libvirt-sandbox-util.c -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] vz: Reformat
25.06.2015 16:21, Michal Privoznik пишет: On 25.06.2015 11:46, Maxim Nestratov wrote: 25.06.2015 12:19, Maxim Nestratov пишет: 25.06.2015 11:18, Michal Privoznik пишет: There shouldn't be any functional change. Only some formatting nits fixed. Michal Privoznik (5): vz_driver: Reformat vz_network: Reformat vz_sdk: Reformat vz_storage: Reformat vz_utils: Reformat src/vz/vz_driver.c | 86 ++--- src/vz/vz_network.c | 26 +++ src/vz/vz_sdk.c | 209 ++-- src/vz/vz_storage.c | 58 +++ src/vz/vz_utils.h | 20 ++--- 5 files changed, 199 insertions(+), 200 deletions(-) ACK. Michal, thank you for this work. Maxim Nestratov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list By the way, Michal, how did you spot all these nits? Is there any tool to do it automatically? make syntax-check doesn't seem to be able to catch all this stuff. I've just noticed that there are some irregularities in the code. So I've opened in in vim and run reformat over the whole files. (gg shift+V shift+g =) Then I've fixed some small nits that vim formatted badly. Michal I see. Thanks again. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] vz: Reformat
On 25.06.2015 11:46, Maxim Nestratov wrote: 25.06.2015 12:19, Maxim Nestratov пишет: 25.06.2015 11:18, Michal Privoznik пишет: There shouldn't be any functional change. Only some formatting nits fixed. Michal Privoznik (5): vz_driver: Reformat vz_network: Reformat vz_sdk: Reformat vz_storage: Reformat vz_utils: Reformat src/vz/vz_driver.c | 86 ++--- src/vz/vz_network.c | 26 +++ src/vz/vz_sdk.c | 209 ++-- src/vz/vz_storage.c | 58 +++ src/vz/vz_utils.h | 20 ++--- 5 files changed, 199 insertions(+), 200 deletions(-) ACK. Michal, thank you for this work. Maxim Nestratov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list By the way, Michal, how did you spot all these nits? Is there any tool to do it automatically? make syntax-check doesn't seem to be able to catch all this stuff. I've just noticed that there are some irregularities in the code. So I've opened in in vim and run reformat over the whole files. (gg shift+V shift+g =) Then I've fixed some small nits that vim formatted badly. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 03/10] Add disk parameter to virt-sandbox
On Thu, 2015-06-25 at 14:09 +0100, Daniel P. Berrange wrote: On Thu, Jun 25, 2015 at 01:27:23PM +0200, Eren Yagdiran wrote: Allow users to add disk images to their sandbox. Only disk images are supported so far, but the parameter is intentionally designed for future changes. --- bin/virt-sandbox.c | 37 + 1 file changed, 37 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) { GMainLoop *loop = NULL; GError *error = NULL; gchar *name = NULL; +gchar **disks = NULL; gchar **mounts = NULL; gchar **includes = NULL; gchar *includefile = NULL; @@ -92,6 +93,8 @@ int main(int argc, char **argv) { N_(name of the sandbox), NAME }, { root, 'r', 0, G_OPTION_ARG_STRING, root, N_(root directory of the sandbox), DIR }, +{ disk, ' ', 0, G_OPTION_ARG_STRING_ARRAY, disks, + N_(add a disk in the guest), TYPE:TARGET=SOURCE,FORMAT=FORMAT }, { mount, 'm', 0, G_OPTION_ARG_STRING_ARRAY, mounts, N_(mount a filesystem in the guest), TYPE:TARGET=SOURCE }, { include, 'i', 0, G_OPTION_ARG_STRING_ARRAY, includes, @@ -182,6 +185,13 @@ int main(int argc, char **argv) { gvir_sandbox_config_set_username(cfg, root); } +if (disks +!gvir_sandbox_config_add_disk_strv(cfg, disks, error)) { +g_printerr(_(Unable to parse disks: %s\n), + error error-message ? error-message : _(Unknown failure)); +goto cleanup; +} + if (mounts !gvir_sandbox_config_add_mount_strv(cfg, mounts, error)) { g_printerr(_(Unable to parse mounts: %s\n), @@ -319,6 +329,33 @@ inheriting the host's root filesystem. NB. CDIR must contain a matching install of the libvirt-sandbox package. This restriction may be lifted in a future version. +=item B--disk TYPE:TARGET=SOURCE,FORMAT=FORMAT + +Sets up a disk inside the sandbox by using BSOURCE with target device BTARGET +and type BTYPE and format BFORMAT. Example: file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 +Format is an optional parameter. + +=over 4 + +=item BTYPE + +Type parameter can be set to file. + +=item BTARGET + +Target parameter can be set to hda or any other libvirt supported target disk Per the discussion on the previous set of patches, we must *not* expose the libvirt disk target device string to the end user at all, because it means the user has to know about the differences in virtualizaton technology used. The latter patches in your series support the /dev/disk/by-tag/TAGNAME symlink to a /dev/TARGET device node, so the user should only be providing the TAGNAMNE and not the TARGET. ie, -disk file:data=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 would end up creating a link /dev/disk/by-tag/data which symlinked to /dev/vda on KVM, or /dev/sda on LXC, or /dev/xvda on Xen (hypothetical) etc. Ooops, I forgot that one when reviewing. The /dev/disk/by-tag/XXX is implemented, the man wasn't changed accordingly. I can change that before pushing if that is the only problem in the series. -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 02/10] Add configuration object for disk support
Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2 --- libvirt-sandbox/Makefile.am | 2 + libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 +++ libvirt-sandbox/libvirt-sandbox-config.c | 293 ++ libvirt-sandbox/libvirt-sandbox-config.h | 10 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 4 + libvirt-sandbox/tests/test-config.c | 11 + 8 files changed, 677 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 6917f04..8237c50 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -55,6 +55,7 @@ SANDBOX_HEADER_FILES = \ libvirt-sandbox-main.h \ libvirt-sandbox-util.h \ libvirt-sandbox-config.h \ + libvirt-sandbox-config-disk.h \ libvirt-sandbox-config-network.h \ libvirt-sandbox-config-network-address.h \ libvirt-sandbox-config-network-filterref-parameter.h \ @@ -86,6 +87,7 @@ SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ libvirt-sandbox-util.c \ libvirt-sandbox-config.c \ + libvirt-sandbox-config-disk.c \ libvirt-sandbox-config-network.c \ libvirt-sandbox-config-network-address.c \ libvirt-sandbox-config-network-filterref.c \ diff --git a/libvirt-sandbox/libvirt-sandbox-config-disk.c b/libvirt-sandbox/libvirt-sandbox-config-disk.c new file mode 100644 index 000..04f1cf0 --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-config-disk.c @@ -0,0 +1,274 @@ +/* + * libvirt-sandbox-config-disk.c: libvirt sandbox configuration + * + * Copyright (C) 2015 Universitat Polit??cnica de Catalunya. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran erenyagdi...@gmail.com + */ + +#include config.h +#include string.h + +#include libvirt-sandbox/libvirt-sandbox.h + +/** + * SECTION: libvirt-sandbox-config-disk + * @short_description: Disk attachment configuration details + * @include: libvirt-sandbox/libvirt-sandbox.h + * @see_aloso: #GVirSandboxConfig + * + * Provides an object to store information about a disk attachment in the sandbox + * + */ + +#define GVIR_SANDBOX_CONFIG_DISK_GET_PRIVATE(obj) \ +(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_SANDBOX_TYPE_CONFIG_DISK, GVirSandboxConfigDiskPrivate)) + + +struct _GVirSandboxConfigDiskPrivate +{ +GVirConfigDomainDiskType type; +gchar *target; +gchar *source; +GVirConfigDomainDiskFormat format; +}; + +G_DEFINE_TYPE(GVirSandboxConfigDisk, gvir_sandbox_config_disk, G_TYPE_OBJECT); + + +enum { +PROP_0, +PROP_TYPE, +PROP_TARGET, +PROP_SOURCE, +PROP_FORMAT +}; + +enum { +LAST_SIGNAL +}; + + + +static void gvir_sandbox_config_disk_get_property(GObject *object, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ +GVirSandboxConfigDisk *config = GVIR_SANDBOX_CONFIG_DISK(object); +GVirSandboxConfigDiskPrivate *priv = config-priv; + +switch (prop_id) { +case PROP_TARGET: +g_value_set_string(value, priv-target); +break; +case PROP_SOURCE: +g_value_set_string(value, priv-source); +break; +case PROP_TYPE: +g_value_set_enum(value, priv-type); +break; +case PROP_FORMAT: +g_value_set_enum(value, priv-format); +break; +default: +G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); +} +} + + +static void
Re: [libvirt] [sandbox PATCH 00/10] Patches for Libvirt-sandbox
On Thu, Jun 25, 2015 at 01:27:20PM +0200, Eren Yagdiran wrote: Hello, These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. --Main diffs compared to previous patches. Since many hypervisors, including kvm, will not even honour requested names for disk devices we link each device under /dev/disk/by-tag e.g /dev/disk/by-tag/foobar - /dev/sda Right, but several patches in this series still deal with allowing the user to specify the /dev/sda name it appears. We populate disks.cfg with tag to device mapping when we build the sandbox. After that, in each init-process {Common,Qemu}, we basically read the configuration and populate the right symlinks under /dev/disk/by-tag The common functions for modifying directories are moved under Init-util. {Common,Qemu} inits are using them. On a general point, can you make sure to run 'make syntax-check' on every patch - there are style mistakes on many of these patches that would break the syntax check Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
On Thu, Jun 25, 2015 at 08:44:17AM +0200, Martin Kletzander wrote: On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote: At this point you can see that it all comes down to what name we want to give the subelement; within that, we give the exact name of the qemu device, and the exact name/value of any qemu options that we set to non-default values. Somebody *please* have an opinion on the name for this, because none of these strikes me as better or worse (well, I think I dislike driver To be honest, I kinds dislike all of them. Not that they would be chosen poorly, no, it's simply because the good sensible choice is unavailable due to another poor decision in the past (this may be another point for Michal's talk on KVM Forum). Thinking about it I must say I don't like how target (which is supposed to match a place where the device appears for the guest) is used for the model specification, on the other hand (ab)using 'model' element for the specification of an address in guest (that's what I understand chassis and port are) doesn't feel any better. What if we go with two of those elements? Would that be too much pain? E.g.: controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' model type='ioh3420'/ target chassis='3' port='0x21'/ /controller I like this, essentially a subModel without the camelCase. One small nit: model name='ioh4320'/ would look nicer to me, but we're using model type=/ in at least one other place, so I'm fine with both. Jan I understand this might look like an overkill, but I think it's better safe then sorry, I guess I just see us not so far in the future regretting any decision made now. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Introduce shadow vol for refresh while the main vol builds.
Hi Jan, Thanks for the review comments. On Tuesday 23 June 2015 06:21 PM, Ján Tomko wrote: On Mon, Jun 22, 2015 at 05:07:26PM +0530, Prerna Saxena wrote: From: Prerna Saxena pre...@linux.vnet.ibm.com Date: Thu, 18 Jun 2015 05:05:09 -0500 Libvirt periodically refreshes all volumes in a storage pool, including the volumes being cloned. While cloning a storage volume from parent, we drop pool locks. Subsequent volume refresh sometimes changes allocation for an ongoing copy, and leads to corrupt images. Fix: Introduce a shadow volume that isolates the volume object under refresh from the base which has a copy ongoing. Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com --- src/storage/storage_driver.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 57060ab..4980546 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1898,7 +1898,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, { virStoragePoolObjPtr pool, origpool = NULL; virStorageBackendPtr backend; -virStorageVolDefPtr origvol = NULL, newvol = NULL; +virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; virStorageVolPtr ret = NULL, volobj = NULL; unsigned long long allocation; int buildret; @@ -2010,6 +2010,17 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (backend-createVol(obj-conn, pool, newvol) 0) goto cleanup; +/* Make a shallow copy of the 'defined' volume definition, since the + * original allocation value will change as the user polls 'info', + * but we only need the initial requested values + */ +if (VIR_ALLOC(shadowvol) 0) { +newvol = NULL; newvol has not been added to pool-volumes.objs yet, so we should free it on the cleanup path. shadowvol should also be VIR_FREE'd. Thanks, I'd missed this. Will be addressed in subsequent patch. +goto cleanup; +} + +memcpy(shadowvol, newvol, sizeof(*newvol)); + pool-volumes.objs[pool-volumes.count++] = newvol; volobj = virGetStorageVol(obj-conn, pool-def-name, newvol-name, newvol-key, NULL, NULL); @@ -2029,7 +2040,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, virStoragePoolObjUnlock(origpool); } -buildret = backend-buildVolFrom(obj-conn, pool, newvol, origvol, flags); +buildret = backend-buildVolFrom(obj-conn, pool, shadowvol, origvol, flags); A few lines below, there is one more usage of newvol after the pool has been unlocked: newvol-target.allocation If the parallel volume refresh happened when the volume was not fully allocated, this might not contain the intended allocation. Using shadowvol-target.allocation will behave the same regardless of a parallel refresh (even though the buildVolFrom function might not honor the allocation exactly). Right. The buildVolFrom() call completes an fsync and then returns. In my experimental runs, a parallel refresh would always happen by the time I got to this point; so this had missed me. But ofcourse we can never say that for sure. Will fix in the next patch. -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Storage : Fix cloning of raw, sparse volumes
On Tuesday 23 June 2015 06:29 PM, Ján Tomko wrote: On Mon, Jun 22, 2015 at 05:09:18PM +0530, Prerna Saxena wrote: From: Prerna Saxena pre...@linux.vnet.ibm.com Date: Mon, 22 Jun 2015 02:54:32 -0500 When virsh vol-clone is attempted on a raw file where capacity allocation, the resulting cloned volume has a size that matches the virtual-size of the parent; in place of matching its actual, disk size. This patch fixes the cloned disk to have same _allocated_size_ as the parent file from which it was cloned. Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739 Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com --- src/storage/storage_backend.c | 10 +- src/storage/storage_driver.c | 5 - 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ce59f63..c99b718 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -342,7 +342,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } -remain = vol-target.allocation; +remain = vol-target.capacity; if (inputvol) { int res = virStorageBackendCopyToFD(vol, inputvol, @@ -397,7 +397,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, bool reflink_copy) { -bool need_alloc = true; +bool need_alloc = !(inputvol (inputvol-target.capacity inputvol-target.allocation)); Comparing 'inputvol-target.capacity vol-target.allocation' would allow creating a sparse volume from a non-sparse one and vice-versa by specifying the correct allocation. Ok, I was not sure if libvirt wanted to do that. If the parent volume is a sparse volume, I'd expect the cloned volume to be sparse too. Likewise, for a non-sparse parent, the cloned volume should also be non-sparse. Should that not be something we honour in libvirt, when we clone ? int ret = 0; unsigned long long remain; @@ -420,7 +420,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, * to writing zeroes block by block in case fallocate isn't * available, and since we're going to copy data from another * file it doesn't make sense to write the file twice. */ -if (vol-target.allocation) { +if (vol-target.allocation need_alloc) { if (fallocate(fd, 0, 0, vol-target.allocation) == 0) { need_alloc = false; } else if (errno != ENOSYS errno != EOPNOTSUPP) { @@ -433,14 +433,14 @@ createRawFile(int fd, virStorageVolDefPtr vol, } #endif -remain = vol-target.allocation; +remain = vol-target.capacity; if (inputvol) { /* allow zero blocks to be skipped if we've requested sparse * allocation (allocation capacity) or we have already * been able to allocate the required space. */ bool want_sparse = !need_alloc || -(vol-target.allocation inputvol-target.capacity); +(inputvol-target.allocation inputvol-target.capacity); If allocation capacity, then need_alloc is already false. I was trying to accomodate the original usecase of need_alloc, but you are right. This will go away in the next version of this series, which I will post shortly. -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Bug: vnc + websocket = websocket autoport not working right at live migration
On Tue, Jun 23, 2015 at 02:13:21PM +0200, Piotr Rybicki wrote: Hello. Problem description: When i start qemu via libvirt with vnc websocket defined, it is not possible to live migrate to host where other qemu process is running with the same display id. migration error is: error: internal error: early end of file from monitor: possible problem: [1] = 2015-06-23T11:54:25.590506Z qemu-system-x86_64: -vnc 0.0.0.0:1,websocket=5700,password: Failed to start VNC server on `(null)': Failed to bind socket: Address already in use (please note vnc display id=1 and websocket=5700 - where it should be 5701) in libvirt's xml i have: (...) graphics type='vnc' port='-1' autoport='yes' websocket='-1' listen='0.0.0.0' passwd='xxx' listen type='address' address='0.0.0.0'/ /graphics (...) for first and only qemu process on host, this creates qemu commandline: (...) -vnc 0.0.0.0:0,websocket=5700,password (...) for second qemu process on the same host: (...) -vnc 0.0.0.0:1,websocket=5701,password (...) There is no problem with migration, when there is no websocket configuration. Solution: I believe, to solve this problem, libvirt has to omit websocket port definition in commandline string ('websocket=5700' = 'websocket') when autoport is defined in domain xml definition. Well, either that or increasing the websocket number as well. And that port should be auto-allocated so in case there is something listening on port 5701 it can select 5702. That would be even more error-proof and libvirt would retain full control of qemu (we do that so that in case the 'websocket = display + 5700' default gets changed, we still know all the details we set up). from man qemu: -vnc display[,option[,option[,...]]] (...) websocket Opens an additional TCP listening port dedicated to VNC Websocket connections. By definition the Websocket port is 5700+display. If host is specified connections will only be allowed from this host. As an alternative the Websocket port could be specified by using websocket=port. TLS encryption for the Websocket connection is supported if the required certificates are specified with the VNC option x509. So if I understand it right, not specifying websocket port means 5700+display id, and display id is given via commandline and increments just fine. Can anyone confirm this? Or perhaps there is some misconfiguration in my xml domain definition? I believe this is a bug in libvirt. Would you mind creating a bug in bugzilla for this issue so we can properly track the issue? Thanks, Martin Best regards Piotr Rybicki -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] docs: Properly mark acl.html dependencies
On Wed, Jun 24, 2015 at 12:44:37PM +0200, Martin Kletzander wrote: On Wed, Jun 24, 2015 at 08:53:21AM +0200, Peter Krempa wrote: On Tue, Jun 23, 2015 at 14:46:36 +0200, Martin Kletzander wrote: On Tue, Jun 23, 2015 at 01:56:00PM +0200, Michal Privoznik wrote: The acl.html file includes aclperms.htmlinc which is generated. However, acl.html is generated too from acl.html.tmp. And in fact, this is the place where the aclperms file is needed. Fix the dependency in Makefile. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- ACK (I thought I ACKed the previous one with this modification, I don't know why :). diff to v1: - Fix the origin of the error docs/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index b7b49cb..13dddf8 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -163,7 +163,7 @@ EXTRA_DIST= \ sitemap.html.in aclperms.htmlinc \ todo.pl hvsupport.pl todo.cfg-example -acl.html:: $(srcdir)/aclperms.htmlinc +acl.html.tmp: $(srcdir)/aclperms.htmlinc Now after a build I get: $ git status On branch master Your branch is up-to-date with 'origin/master'. Untracked files: (use git add file... to include in what will be committed) docs/acl.html.tmp nothing added to commit but untracked files present (use git add to track) Aha! I guess that's why the double colon must be there, because it sets one dependency and then there are bunch of other rules that I haven't found the first time (using wildcards), like '%.html: %.html.tmp' Weird that I don't see that after build... Now I see that. And I probably figured out why. No double colon has anything to do with it. And it's probably the reason why there was acl.html specified instead of acl.html.tmp. the thing is that if you have: filename: another.file in the Makefile and you run make, it will keep that immediate file in place, but if the immediate file is specified using wildcards, e.g.: %name: another.file make will clean that up after using it. I couldn't find why it does that and whether that's documented behaviour, but it certainly is something I didn't expect. You can try that this is fixed by changing: acl.html.tmp: $(srcdir)/aclperms.htmlinc to %acl.html.tmp: $(srcdir)/aclperms.htmlinc Having said that, I know that is not the right fix, but at such late hour and low caffeine/blood ratio I can't think of anything else than either this or having all html.tmp files depending on aclperms.htmlinc. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix not format priority when it is zero
On Wed, Jun 24, 2015 at 12:00:36PM +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1235116 We do not format the priority if it is 0, but this will be a broken settings in guest. Change the condition of format priority element to always format priority if scheduler is 'fifo' or 'rr'. Signed-off-by: Luyao Huang lhu...@redhat.com --- I haven't intorduce a new bool parameter to mark if we set the priority value just like the other place we avoid this issue, because i think this looks unnecessary in this place. src/conf/domain_conf.c | 6 ++-- The part for domain_conf.c didn't apply properly, but it's easy enough to fix. I modified the commit message as follows and pushed the patch: conf: Format scheduler priority when it is zero According to our XML definition, zero is as valid as any other value. Mainly because it should be kernel-agnostic. Martin signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Plans for next release
Sorry I nearly forgot, but we should enter freeze soon to try to hit the end of the month target. I see that Martin's APIs for admin support has been merged in so that would be libvirt-1.3.0. Plan is to enter freeze this w.e. and try to release by the end of the week, I hope this works for everyone, thanks, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Handle isa-fdc removal from new q35 machine types
On 06/22/2015 09:57 AM, Ján Tomko wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1227880 Ján Tomko (2): Separate isa-fdc options generation Explicitly format the isa-fdc controller for newer q35 machines src/qemu/qemu_command.c| 50 -- src/qemu/qemu_domain.c | 22 ++ src/qemu/qemu_domain.h | 1 + .../qemuxml2argv-boot-floppy-q35.args | 12 ++ .../qemuxml2argv-boot-floppy-q35.xml | 40 + .../qemuxml2argv-bootindex-floppy-q35.args | 11 + .../qemuxml2argv-bootindex-floppy-q35.xml | 40 + tests/qemuxml2argvtest.c | 9 8 files changed, 172 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootindex-floppy-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootindex-floppy-q35.xml ACK series. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] vz: add net dev statistiscs
On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote: From: Nikolay Shirokovskiy nshirokovs...@parallels.com Populate counters SDK currenly supports: rx_bytes rx_packets tx_bytes tx_packets Comments. 1. Use vzDomObjFromDomainRef/virDomainObjEndAPI pair to get domain object as we use prlsdkGetStatsParam that can release domain object lock and thus we need a reference in case domain is deleated meanwhile. 2. Introduce prlsdkGetAdapterIndex to convert from device name to SDK device index. We need this index as it is used in statistics data from SDK identify network device. Unfortunately we need to enumerate network devices to discover this mapping. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/vz/vz_driver.c | 44 + src/vz/vz_sdk.c| 69 +++- src/vz/vz_sdk.h|4 +++ 3 files changed, 116 insertions(+), 1 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index cef3c77..deac572 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1321,6 +1321,49 @@ vzDomainBlockStatsFlags(virDomainPtr domain, return ret; } +static int +vzDomainInterfaceStats(virDomainPtr domain, + const char *path, + virDomainInterfaceStatsPtr stats) +{ +virDomainObjPtr dom = NULL; +int ret = -1; +int net_index = -1; +char *name = NULL; + +if (!(dom = vzDomObjFromDomainRef(domain))) +return -1; + +net_index = prlsdkGetAdapterIndex(dom, path); We have function prlsdkGetNetIndex, which looks up an adapter by given libvirt structure virDomainNetDef. Net and Adapter are sysnonyms, libvirt uses Net, and prlsdk uses 'Adapter' name, so I think we should rename these function, so the name will show, by which property adapter is looked up. +if (net_index 0) In libvirt's code people usually combine the function call and a check in one line if ((net_index = prlsdkGetAdapterIndex(dom, path)) 0) ... +return -1; + +#define PARALLELS_GET_NET_COUNTER(VAL, NAME)\ +if (virAsprintf(name, net.nic%d.%s, net_index, NAME) 0)\ +goto cleanup; \ +if (prlsdkGetStatsParam(dom, name, stats-VAL) 0)\ +goto cleanup; \ +VIR_FREE(name); + +PARALLELS_GET_NET_COUNTER(rx_bytes, bytes_in) +PARALLELS_GET_NET_COUNTER(rx_packets, pkts_in) +PARALLELS_GET_NET_COUNTER(tx_bytes, bytes_out) +PARALLELS_GET_NET_COUNTER(tx_packets, pkts_out) +stats-rx_errs = -1; +stats-rx_drop = -1; +stats-tx_errs = -1; +stats-tx_drop = -1; + +#undef PARALLELS_GET_NET_COUNTER +ret = 0; + + cleanup: +VIR_FREE(name); +if (dom) +virDomainObjEndAPI(dom); + +return ret; +} static virHypervisorDriver vzDriver = { .name = vz, @@ -1373,6 +1416,7 @@ static virHypervisorDriver vzDriver = { .domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */ .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */ .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */ +.domainInterfaceStats = vzDomainInterfaceStats, /* 1.3.0 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index f9cde44..0956b58 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3562,7 +3562,7 @@ prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char *name, long long *val) #define PARALLELS_STATISTICS_TIMEOUT (60 * 1000) -static int +int prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val) { vzDomObjPtr privdom = dom-privateData; @@ -3658,3 +3658,70 @@ prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBloc VIR_FREE(name); return ret; } + +static PRL_HANDLE +prlsdkFindNetAdapter(virDomainObjPtr dom, const char *path) +{ +PRL_UINT32 count = 0; +vzDomObjPtr privdom = dom-privateData; +PRL_UINT32 buflen = 0; +PRL_RESULT pret; +size_t i; +char *name = NULL; +PRL_HANDLE net = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetNetAdaptersCount(privdom-sdkdom, count); +prlsdkCheckRetGoto(pret, error); + +for (i = 0; i count; ++i) { +pret = PrlVmCfg_GetNetAdapter(privdom-sdkdom, i, net); +prlsdkCheckRetGoto(pret, error); + +pret = PrlVmDevNet_GetHostInterfaceName(net, NULL, buflen); +prlsdkCheckRetGoto(pret, error); + +if (VIR_ALLOC_N(name, buflen) 0) +goto error; + +pret = PrlVmDevNet_GetHostInterfaceName(net, name, buflen); +prlsdkCheckRetGoto(pret, error); + +if (STREQ(name, path)) +break; + +VIR_FREE(name); +PrlHandle_Free(net); +net = PRL_INVALID_HANDLE; +} + +if (net == PRL_INVALID_HANDLE) +virReportError(VIR_ERR_INVALID_ARG, +
Re: [libvirt] [PATCH v2 1/3] vz: add net dev statistiscs
On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote: From: Nikolay Shirokovskiy nshirokovs...@parallels.com Populate counters SDK currenly supports: rx_bytes rx_packets tx_bytes tx_packets Comments. 1. Use vzDomObjFromDomainRef/virDomainObjEndAPI pair to get domain object as we use prlsdkGetStatsParam that can release domain object lock and thus we need a reference in case domain is deleated meanwhile. 2. Introduce prlsdkGetAdapterIndex to convert from device name to SDK device index. We need this index as it is used in statistics data from SDK identify network device. Unfortunately we need to enumerate network devices to discover this mapping. Also this patch doesn't apply at this moment. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/vz/vz_driver.c | 44 + src/vz/vz_sdk.c| 69 +++- src/vz/vz_sdk.h|4 +++ 3 files changed, 116 insertions(+), 1 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index cef3c77..deac572 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1321,6 +1321,49 @@ vzDomainBlockStatsFlags(virDomainPtr domain, return ret; } +static int +vzDomainInterfaceStats(virDomainPtr domain, + const char *path, + virDomainInterfaceStatsPtr stats) +{ +virDomainObjPtr dom = NULL; +int ret = -1; +int net_index = -1; +char *name = NULL; + +if (!(dom = vzDomObjFromDomainRef(domain))) +return -1; + +net_index = prlsdkGetAdapterIndex(dom, path); +if (net_index 0) +return -1; + +#define PARALLELS_GET_NET_COUNTER(VAL, NAME)\ +if (virAsprintf(name, net.nic%d.%s, net_index, NAME) 0)\ +goto cleanup; \ +if (prlsdkGetStatsParam(dom, name, stats-VAL) 0)\ +goto cleanup; \ +VIR_FREE(name); + +PARALLELS_GET_NET_COUNTER(rx_bytes, bytes_in) +PARALLELS_GET_NET_COUNTER(rx_packets, pkts_in) +PARALLELS_GET_NET_COUNTER(tx_bytes, bytes_out) +PARALLELS_GET_NET_COUNTER(tx_packets, pkts_out) +stats-rx_errs = -1; +stats-rx_drop = -1; +stats-tx_errs = -1; +stats-tx_drop = -1; + +#undef PARALLELS_GET_NET_COUNTER +ret = 0; + + cleanup: +VIR_FREE(name); +if (dom) +virDomainObjEndAPI(dom); + +return ret; +} static virHypervisorDriver vzDriver = { .name = vz, @@ -1373,6 +1416,7 @@ static virHypervisorDriver vzDriver = { .domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */ .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */ .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */ +.domainInterfaceStats = vzDomainInterfaceStats, /* 1.3.0 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index f9cde44..0956b58 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3562,7 +3562,7 @@ prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char *name, long long *val) #define PARALLELS_STATISTICS_TIMEOUT (60 * 1000) -static int +int prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val) { vzDomObjPtr privdom = dom-privateData; @@ -3658,3 +3658,70 @@ prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBloc VIR_FREE(name); return ret; } + +static PRL_HANDLE +prlsdkFindNetAdapter(virDomainObjPtr dom, const char *path) +{ +PRL_UINT32 count = 0; +vzDomObjPtr privdom = dom-privateData; +PRL_UINT32 buflen = 0; +PRL_RESULT pret; +size_t i; +char *name = NULL; +PRL_HANDLE net = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetNetAdaptersCount(privdom-sdkdom, count); +prlsdkCheckRetGoto(pret, error); + +for (i = 0; i count; ++i) { +pret = PrlVmCfg_GetNetAdapter(privdom-sdkdom, i, net); +prlsdkCheckRetGoto(pret, error); + +pret = PrlVmDevNet_GetHostInterfaceName(net, NULL, buflen); +prlsdkCheckRetGoto(pret, error); + +if (VIR_ALLOC_N(name, buflen) 0) +goto error; + +pret = PrlVmDevNet_GetHostInterfaceName(net, name, buflen); +prlsdkCheckRetGoto(pret, error); + +if (STREQ(name, path)) +break; + +VIR_FREE(name); +PrlHandle_Free(net); +net = PRL_INVALID_HANDLE; +} + +if (net == PRL_INVALID_HANDLE) +virReportError(VIR_ERR_INVALID_ARG, + _(invalid path, '%s' is not a known interface), path); +return net; + + error: +VIR_FREE(name); +PrlHandle_Free(net); +return PRL_INVALID_HANDLE; +} + +int +prlsdkGetAdapterIndex(virDomainObjPtr dom, const char *path) +{ +PRL_HANDLE net = PRL_INVALID_HANDLE; +PRL_UINT32 net_index = -1; +PRL_RESULT pret; +int ret = -1; + +net = prlsdkFindNetAdapter(dom,
Re: [libvirt] [PATCH 1/5] internal: Introduce virCheckNonEmptyStringArgGoto and reuse it
On 06/23/2015 01:15 PM, Peter Krempa wrote: The helper makes sure that strings passed to APIs are non-NULL and non-empty. This allows to drop some inlined checks where it does not make sense. --- src/internal.h | 11 +++ src/libvirt-domain.c | 4 ++-- src/qemu/qemu_driver.c | 11 --- src/util/virerror.h| 11 +++ 4 files changed, 24 insertions(+), 13 deletions(-) ... diff --git a/src/util/virerror.h b/src/util/virerror.h index ad3a946..c1a445e 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -95,6 +95,17 @@ void virReportSystemErrorFull(int domcode, 0, 0, \ _(%s in %s must not be NULL),\ #argname, __FUNCTION__) +# define virReportInvalidEmptyStringArg(argname) \ +virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ + VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG, \ + VIR_ERR_ERROR, \ + __FUNCTION__, \ + #argname, \ + NULL, \ + 0, 0, \ + _(string %s in %s must not be non empty),\ This reads strangely... The double negative (not and non) string %s in %s must not be empty or string %s in %s must be non empty Or more technically string %s in %s cannot start with the NUL character (yes, correct spelling for NUL - that's the '\0') + #argname, __FUNCTION__) # define virReportInvalidPositiveArg(argname)\ virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ VIR_FROM_THIS, \ ACK with some sort of change John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 00/10] Actually use host-image mounts on /
Hi all, In the virt-sandbox documentation we had examples with host-image mounts targetting /. However the / in the sandbox was still the host one. The main goal of this patch series is to fix that problem. This will also be needed to run docker container with libvirt-sandbox. I also added some configure flags to disable lzma or zlib support at build time. At least opensuse 13.2 doesn't have static lzma. Cédric Bosdonnat (10): Allow disabling build with lzma. Allow disabling zlib support. Enable strcmp checks in libvirt-sandbox-init-qemu.c Copy init-common and all its deps to config subdir Remove init-common dependency on libvirt-sandbox.so init-qemu: extract the mounts.cfg entry mounting code qemu: use mounts targeting / as root Add function to check if there is a mount with / target Don't add sandbox:root device if we have a mount targetting / container builder: don't expose host rootfs if unneeded cfg.mk | 2 +- configure.ac | 37 - libvirt-sandbox/Makefile.am| 41 +++-- .../libvirt-sandbox-builder-container.c| 22 +-- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 136 +++-- libvirt-sandbox/libvirt-sandbox-config-all.h | 61 .../libvirt-sandbox-config-interactive.c | 2 +- .../libvirt-sandbox-config-mount-file.c| 2 +- .../libvirt-sandbox-config-mount-guest-bind.c | 2 +- .../libvirt-sandbox-config-mount-host-bind.c | 2 +- .../libvirt-sandbox-config-mount-host-image.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-mount-ram.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-mount.c | 2 +- .../libvirt-sandbox-config-network-address.c | 2 +- ...rt-sandbox-config-network-filterref-parameter.c | 2 +- .../libvirt-sandbox-config-network-filterref.c | 2 +- .../libvirt-sandbox-config-network-route.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-network.c | 2 +- .../libvirt-sandbox-config-service-generic.c | 2 +- .../libvirt-sandbox-config-service-systemd.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-service.c | 2 +- libvirt-sandbox/libvirt-sandbox-config.c | 23 ++- libvirt-sandbox/libvirt-sandbox-config.h | 1 + libvirt-sandbox/libvirt-sandbox-init-common.c | 5 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c| 166 + libvirt-sandbox/libvirt-sandbox.h | 18 +-- libvirt-sandbox/libvirt-sandbox.sym| 1 + 27 files changed, 442 insertions(+), 101 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-all.h -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 09/10] Don't add sandbox:root device if we have a mount targetting /
There is no need to expose the host file system if the user defined a mount targetting / --- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 7a2af83..cd459ac 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -590,17 +590,19 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde construct_devices(builder, config, statedir, domain, error)) goto cleanup; -fs = gvir_config_domain_filesys_new(); -gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); -gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_SQUASH); -gvir_config_domain_filesys_set_source(fs, - gvir_sandbox_config_get_root(config)); -gvir_config_domain_filesys_set_target(fs, sandbox:root); -gvir_config_domain_filesys_set_readonly(fs, TRUE); +if (!gvir_sandbox_config_has_root_mount(config)) { +fs = gvir_config_domain_filesys_new(); +gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); +gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_SQUASH); +gvir_config_domain_filesys_set_source(fs, + gvir_sandbox_config_get_root(config)); +gvir_config_domain_filesys_set_target(fs, sandbox:root); +gvir_config_domain_filesys_set_readonly(fs, TRUE); -gvir_config_domain_add_device(domain, - GVIR_CONFIG_DOMAIN_DEVICE(fs)); -g_object_unref(fs); +gvir_config_domain_add_device(domain, + GVIR_CONFIG_DOMAIN_DEVICE(fs)); +g_object_unref(fs); +} fs = gvir_config_domain_filesys_new(); -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 10/10] container builder: don't expose host rootfs if unneeded
If the user defined a mount targeting / don't add the host / as mount to /. --- .../libvirt-sandbox-builder-container.c| 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c index bd29c87..8315ab5 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c @@ -225,17 +225,19 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil construct_devices(builder, config, statedir, domain, error)) goto cleanup; -fs = gvir_config_domain_filesys_new(); -gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); -gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_PASSTHROUGH); -gvir_config_domain_filesys_set_source(fs, - gvir_sandbox_config_get_root(config)); -gvir_config_domain_filesys_set_target(fs, /); -gvir_config_domain_filesys_set_readonly(fs, TRUE); +if (!gvir_sandbox_config_has_root_mount(config)) { +fs = gvir_config_domain_filesys_new(); +gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); +gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_PASSTHROUGH); +gvir_config_domain_filesys_set_source(fs, + gvir_sandbox_config_get_root(config)); +gvir_config_domain_filesys_set_target(fs, /); +gvir_config_domain_filesys_set_readonly(fs, TRUE); -gvir_config_domain_add_device(domain, - GVIR_CONFIG_DOMAIN_DEVICE(fs)); -g_object_unref(fs); +gvir_config_domain_add_device(domain, + GVIR_CONFIG_DOMAIN_DEVICE(fs)); +g_object_unref(fs); +} -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 03/10] Enable strcmp checks in libvirt-sandbox-init-qemu.c
--- cfg.mk | 2 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index 83ded15..e9c12f7 100644 --- a/cfg.mk +++ b/cfg.mk @@ -133,4 +133,4 @@ exclude_file_name_regexp--sc_bindtextdomain = ^(libvirt-sandbox/tests)|(libvirt- exclude_file_name_regexp--sc_preprocessor_indentation = ^*/*.[ch] -exclude_file_name_regexp--sc_prohibit_strcmp = ^libvirt-sandbox/libvirt-sandbox-init-qemu.c +#exclude_file_name_regexp--sc_prohibit_strcmp = ^libvirt-sandbox/libvirt-sandbox-init-qemu.c diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index a20db77..db67fdb 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -51,6 +51,8 @@ #define ATTR_UNUSED __attribute__((__unused__)) +#define STREQ(x,y) (strcmp(x,y) == 0) + static void print_uptime (void); static void insmod (const char *filename); static void set_debug(void); @@ -375,7 +377,7 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) if (strncmp(source, /dev/vd, 7) == 0) create_virtioblk_device(source); -if (strcmp(type, ) == 0) { +if (STREQ(type, )) { struct stat st; type = NULL; flags |= MS_BIND; @@ -389,7 +391,7 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) else mount_mkfile(target, 644); } else { -if (strcmp(type, tmpfs) == 0) +if (STREQ(type, tmpfs)) flags |= MS_NOSUID | MS_NODEV; mount_mkdir(target, 0755); -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 08/10] Add function to check if there is a mount with / target
gvir_sandbox_config_has_root_mount is a convenience function to check if there is a mount with target '/' --- libvirt-sandbox/libvirt-sandbox-config.c | 21 + libvirt-sandbox/libvirt-sandbox-config.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 1 + 3 files changed, 23 insertions(+) diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index d82076a..c467ca8 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -1389,6 +1389,27 @@ gboolean gvir_sandbox_config_has_mounts_with_type(GVirSandboxConfig *config, } +gboolean gvir_sandbox_config_has_root_mount(GVirSandboxConfig *config) +{ +GList *tmp = NULL, *mounts = NULL; +gboolean hasRoot = FALSE; + +tmp = mounts = gvir_sandbox_config_get_mounts(config); +while (tmp !hasRoot) { +const gchar *target; +GVirSandboxConfigMount *mount = GVIR_SANDBOX_CONFIG_MOUNT(tmp-data); +target = gvir_sandbox_config_mount_get_target(mount); +if (g_str_equal(target, /)) +hasRoot = TRUE; +tmp = tmp-next; +} +g_list_foreach(mounts, (GFunc)g_object_unref, NULL); +g_list_free(mounts); + +return hasRoot; +} + + /** diff --git a/libvirt-sandbox/libvirt-sandbox-config.h b/libvirt-sandbox/libvirt-sandbox-config.h index 1a65e3d..0a9ef3b 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.h +++ b/libvirt-sandbox/libvirt-sandbox-config.h @@ -139,6 +139,7 @@ gboolean gvir_sandbox_config_add_mount_strv(GVirSandboxConfig *config, gboolean gvir_sandbox_config_has_mounts(GVirSandboxConfig *config); gboolean gvir_sandbox_config_has_mounts_with_type(GVirSandboxConfig *config, GType type); +gboolean gvir_sandbox_config_has_root_mount(GVirSandboxConfig *config); gboolean gvir_sandbox_config_add_host_include_strv(GVirSandboxConfig *config, gchar **includes, diff --git a/libvirt-sandbox/libvirt-sandbox.sym b/libvirt-sandbox/libvirt-sandbox.sym index a17dfed..dba4068 100644 --- a/libvirt-sandbox/libvirt-sandbox.sym +++ b/libvirt-sandbox/libvirt-sandbox.sym @@ -212,5 +212,6 @@ LIBVIRT_SANDBOX_0.2.1 { LIBVIRT_SANDBOX_0.5.2 { global: +gvir_sandbox_config_has_root_mount; gvir_sandbox_config_mount_guest_bind_get_format; } LIBVIRT_SANDBOX_0.2.1; -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 05/10] Remove init-common dependency on libvirt-sandbox.so
Removing this dependency avoids getting all libvirt.so dependencies loaded in our container. --- libvirt-sandbox/Makefile.am| 41 +++ libvirt-sandbox/libvirt-sandbox-config-all.h | 61 ++ .../libvirt-sandbox-config-interactive.c | 2 +- .../libvirt-sandbox-config-mount-file.c| 2 +- .../libvirt-sandbox-config-mount-guest-bind.c | 2 +- .../libvirt-sandbox-config-mount-host-bind.c | 2 +- .../libvirt-sandbox-config-mount-host-image.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-mount-ram.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-mount.c | 2 +- .../libvirt-sandbox-config-network-address.c | 2 +- ...rt-sandbox-config-network-filterref-parameter.c | 2 +- .../libvirt-sandbox-config-network-filterref.c | 2 +- .../libvirt-sandbox-config-network-route.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-network.c | 2 +- .../libvirt-sandbox-config-service-generic.c | 2 +- .../libvirt-sandbox-config-service-systemd.c | 2 +- libvirt-sandbox/libvirt-sandbox-config-service.c | 2 +- libvirt-sandbox/libvirt-sandbox-config.c | 2 +- libvirt-sandbox/libvirt-sandbox-init-common.c | 5 +- libvirt-sandbox/libvirt-sandbox.h | 18 +-- 20 files changed, 111 insertions(+), 46 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-all.h diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 30c9ebf..0e623c5 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -50,11 +50,9 @@ SANDBOX_RPC_FILES = \ libvirt-sandbox-rpcpacket.h \ $(NULL) -SANDBOX_HEADER_FILES = \ - libvirt-sandbox.h \ - libvirt-sandbox-main.h \ - libvirt-sandbox-util.h \ +SANDBOX_CONFIG_HEADER_FILES = \ libvirt-sandbox-config.h \ + libvirt-sandbox-config-all.h \ libvirt-sandbox-config-network.h \ libvirt-sandbox-config-network-address.h \ libvirt-sandbox-config-network-filterref-parameter.h \ @@ -71,6 +69,12 @@ SANDBOX_HEADER_FILES = \ libvirt-sandbox-config-service.h \ libvirt-sandbox-config-service-systemd.h \ libvirt-sandbox-config-service-generic.h \ + $(NULL) + +SANDBOX_HEADER_FILES = \ + libvirt-sandbox.h \ + libvirt-sandbox-main.h \ + libvirt-sandbox-util.h \ libvirt-sandbox-builder.h \ libvirt-sandbox-builder-initrd.h \ libvirt-sandbox-builder-machine.h \ @@ -81,7 +85,9 @@ SANDBOX_HEADER_FILES = \ libvirt-sandbox-context.h \ libvirt-sandbox-context-interactive.h \ libvirt-sandbox-context-service.h \ + $(SANDBOX_CONFIG_HEADER_FILES) \ $(NULL) + SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ libvirt-sandbox-config.c \ @@ -166,31 +172,48 @@ libvirt_sandbox_1_0_la_LDFLAGS = \ libvirt_sandbox_init_common_SOURCES = libvirt-sandbox-init-common.c \ $(SANDBOX_GENERATED_RPC_FILES) \ $(SANDBOX_RPC_FILES) \ + $(SANDBOX_CONFIG_HEADER_FILES) \ + libvirt-sandbox-config.c \ + libvirt-sandbox-config-network.c \ + libvirt-sandbox-config-network-address.c \ + libvirt-sandbox-config-network-filterref.c \ + libvirt-sandbox-config-network-filterref-parameter.c \ + libvirt-sandbox-config-network-route.c \ + libvirt-sandbox-config-mount.c \ + libvirt-sandbox-config-mount-file.c \ + libvirt-sandbox-config-mount-host-bind.c \ + libvirt-sandbox-config-mount-host-image.c \ + libvirt-sandbox-config-mount-guest-bind.c \ + libvirt-sandbox-config-mount-ram.c \ + libvirt-sandbox-config-interactive.c \ + libvirt-sandbox-config-service.c \ + libvirt-sandbox-config-service-systemd.c \ + libvirt-sandbox-config-service-generic.c \ $(NULL) libvirt_sandbox_init_common_CFLAGS = \ -DLIBEXECDIR=\$(libexecdir)\ \ -DSANDBOXCONFIGDIR=\$(sandboxconfigdir)\ \ -DLOCALEDIR=\$(datadir)/locale\ \ + -DLIBVIRT_SANDBOX_BUILD \ $(COVERAGE_CFLAGS) \
Re: [libvirt] [PATCH v2 2/3] vz: add vcpu statistics
On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote: From: Nikolay Shirokovskiy nshirokovs...@parallels.com Comments. Replace vzDomObjFromDomain/virObjectUnlock pair to vzDomObjFromDomainRef/virDomainObjEndAPI as we use prlsdkGetStatsParam. See previous statistics comments. Looks good to me, ACK. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/vz/vz_driver.c | 16 ++-- 1 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index deac572..4197569 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -825,8 +825,9 @@ vzDomainGetVcpus(virDomainPtr domain, size_t i; int v, maxcpu, hostcpus; int ret = -1; +char *name = NULL; -if (!(privdom = vzDomObjFromDomain(domain))) +if (!(privdom = vzDomObjFromDomainRef(domain))) goto cleanup; if (!virDomainObjIsActive(privdom)) { @@ -847,8 +848,18 @@ vzDomainGetVcpus(virDomainPtr domain, if (info != NULL) { memset(info, 0, sizeof(*info) * maxinfo); for (i = 0; i maxinfo; i++) { +long long vcpuTime = 0; + info[i].number = i; info[i].state = VIR_VCPU_RUNNING; + +if (virAsprintf(name, guest.vcpu%u.time, (unsigned int)i) 0) +goto cleanup; +if (prlsdkGetStatsParam(privdom, name, vcpuTime) 0) +goto cleanup; +if (vcpuTime != -1) +info[i].cpuTime = vcpuTime; +VIR_FREE(name); } } if (cpumaps != NULL) { @@ -871,7 +882,8 @@ vzDomainGetVcpus(virDomainPtr domain, cleanup: if (privdom) -virObjectUnlock(privdom); +virDomainObjEndAPI(privdom); +VIR_FREE(name); return ret; } -- Dmitry Guryanov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_hotplug: try harder to eject media
On 06/23/2015 07:13 AM, Pavel Hrdina wrote: Some guests lock the tray and QEMU eject command will simply fail to eject the media. But the guest OS can handle this attempt to eject the media and can unlock the tray and open it. In this case, we should try again to actually eject the media. If the first attempt fails we will wait for tray to open and try again to eject the media, but if the second attempt fails, returns with error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1147471 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_hotplug.c | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0628964..0cebaad 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -201,13 +201,17 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } if (ret 0) -goto error; +VIR_DEBUG(tray is probably locked, wait for the guest to unlock + the tray and open it); There are more reasons than waiting for the unlock that would cause a negative value to be returned, so this seems dangerous.. !mon if json !cmd qemuMonitorJSONCommand failure qemuMonitorJSONCheckError failure else virAsprintf Failure qemuMonitorHMPCommand failure c_strcasestr failure Is there no way to determine that the failure was due to a locked tray from the error message? The bz notes a DEVICE_TRAY_MOVED event, but I don't see that here. I would think that if this is handle-able from qemu/json only, then the qemuMonitorJSONEjectMedia would be the best place to at least check for this path of retry logic. You'll have the json error returned (per the link in bz comment 3): {error: {class: GenericError, desc: Device 'cd' is locked}} That should be parse-able by this code and either handled back in the caller or in this code. Since qemuMonitorJSONEjectMedia doesn't list return values, perhaps use -1 to mean total failure, 0 to mean pseudo-success as it does today, and 1 to indicate tray was locked and that the caller can/should retry. FWIW: By pseudo-success... I see that if 0 is currently returned, the while loop will look for the tray_status == VIR_DOMAIN_DISK_TRAY_OPEN and fail if not eventually seen Consider the following... bool retry_lock_tray = false; do { qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorEjectMedia(priv-mon, driveAlias, force); if (qemuDomainObjExitMonitor(driver, vm) 0) { ret = -1; goto cleanup; } /* We already tried, but got the locked error again */ if (ret == 1 retry_lock_tray) { some sort of failure since you retried ret = -1; goto error; } /* We were locked, but haven't retried */ if (ret == 1) { Wait some period of time for the DEVICE_TRAY_MOVED event if event found retry_lock_tray = true; } } while (retry_lock_tray); if (ret 0) goto error; if (ret == 1) { ERROR tray locked by guest, never got DEVICE_TRAY_MOVED ret = -1; got error; } virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ while (retries) { -if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) +if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { +ret = 0; +virResetLastError(); break; +} retries--; virObjectUnlock(vm); If it's locked how would this loop ever succeed? That is how would the tray open magically if it's locked and we haven't sent another eject request? @@ -218,10 +222,29 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virObjectUnref(vm); if (retries = 0) { -virReportError(VIR_ERR_OPERATION_FAILED, %s, - _(Unable to eject media)); -ret = -1; +/* Report a new error only if the first attempt don't fail and we don't + * receive VIR_DOMAIN_DISK_TRAY_OPEN event, otherwise report the error + * from first attempt. */ +if (ret == 0) { +virReportError(VIR_ERR_OPERATION_FAILED, %s, + _(Unable to eject media)); +ret = -1; +} goto error; +} else { +/* QEMU will send eject request to guest, but if the tray is locked, + * always returns with error. However the guest can handle the eject + * request, unlock the tray and open it. In case this happens, we + * should try to eject the media once more. */ +qemuDomainObjEnterMonitor(driver, vm); +ret = qemuMonitorEjectMedia(priv-mon, driveAlias, force); +if (qemuDomainObjExitMonitor(driver, vm) 0) { +ret = -1; +
Re: [libvirt] [sandbox PATCH 09/10] Common-init: Building symlink from disks.cfg
On Thu, Jun 25, 2015 at 01:27:29PM +0200, Eren Yagdiran wrote: Similar to the existing mounts.cfg, the mapping between the device and the tag is passed by a new disks.cfg file. Common-init reads disks.cfg and maps the tags to corresponding devices --- libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c b/libvirt-sandbox/libvirt-sandbox-init-common.c index 68f96ba..f8b2ea5 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-common.c +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c @@ -46,6 +46,7 @@ #include grp.h #include libvirt-sandbox-rpcpacket.h +#include libvirt-sandbox-init-util.h static gboolean debug = FALSE; static gboolean verbose = FALSE; @@ -60,6 +61,51 @@ static void sig_child(int sig ATTR_UNUSED) abort(); } +static gboolean setup_disk_tags(void) { +FILE *fp; +gboolean ret = FALSE; +static char line[1024]; +if (debug) +fprintf(stderr, libvirt-sandbox-init-common: %s: populate /dev/disk/by-tag/\n, + __func__); +fp = fopen(SANDBOXCONFIGDIR /disks.cfg, r); +if (fp == NULL) { +fprintf(stderr, libvirt-sandbox-init-common: %s: cannot open SANDBOXCONFIGDIR /disks.cfg: %s\n, +__func__, strerror(errno)); + +goto cleanup; +} +gvir_sandbox_init_util_mkdir(/dev/disk/by-tag, 0755, debug == TRUE ? 1 : 0); +while (fgets(line, sizeof line, fp)) { +char path[1024]; +char *tag = line; +char *device = strchr(tag, '\t'); +*device = '\0'; +device++; +char *tmp = strchr(device, '\n'); +*tmp = '\0'; + +if (sprintf(path, /dev/disk/by-tag/%s, tag) 0) { It is preferrable to use g_strdup_printf() from glib instead of sprintf into a fixed buffer with no overflow checks. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox 01/10] Allow disabling build with lzma.
On Thu, Jun 25, 2015 at 06:49:38PM +0200, Cédric Bosdonnat wrote: Some linux distributions don't package static lzma library. Allow disabling it. --- configure.ac| 14 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 12 2 files changed, 25 insertions(+), 1 deletion(-) ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 01/10] Allow disabling build with lzma.
Some linux distributions don't package static lzma library. Allow disabling it. --- configure.ac| 14 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 12 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 140fb8c..834a444 100644 --- a/configure.ac +++ b/configure.ac @@ -81,7 +81,14 @@ PKG_CHECK_MODULES(LIBVIRT_GLIB, libvirt-glib-1.0 = $LIBVIRT_GOBJECT_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GOBJECT, libvirt-gobject-1.0 = $LIBVIRT_GOBJECT_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GCONFIG, libvirt-gconfig-1.0 = $LIBVIRT_GCONFIG_REQUIRED) PKG_CHECK_MODULES(ZLIB, zlib = $ZLIB_REQUIRED) -PKG_CHECK_MODULES(LZMA, liblzma = $LZMA_REQUIRED) +AC_ARG_WITH([lzma], + [AS_HELP_STRING([--with-lzma], +[add LZMA support @:@default=yes@:@])]) +m4_divert_text([DEFAULTS], [with_lzma=yes]) + +if test $with_lzma = yes ; then +PKG_CHECK_MODULES(LZMA, liblzma = $LZMA_REQUIRED) +fi LIBVIRT_SANDBOX_CAPNG LIBVIRT_SANDBOX_GETTEXT @@ -118,6 +125,11 @@ AC_MSG_NOTICE([]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ Libraries:]) AC_MSG_NOTICE([]) +if test $with_lzma != no ; then +AC_MSG_NOTICE([LZMA: $LZMA_CFLAGS $LZMA_LIBS]) +else +AC_MSG_NOTICE([LZMA: no]) +fi AC_MSG_NOTICE([ GOBJECT: $GOBJECT_CFLAGS $GOBJECT_LIBS]) AC_MSG_NOTICE([ LIBVIRT_GOBJECT: $LIBVIRT_GOBJECT_CFLAGS $LIBVIRT_GOBJECT_LIBS]) AC_MSG_NOTICE([]) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 2c2c803..e91dbcf 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -42,7 +42,9 @@ #include fcntl.h #include sys/reboot.h #include termios.h +#if WITH_LZMA #include lzma.h +#endif /* WITH_LZMA */ #include zlib.h #define ATTR_UNUSED __attribute__((__unused__)) @@ -492,6 +494,7 @@ has_suffix(const char *filename, const char *ext) offset[strlen(ext)] == '\0'); } +#if WITH_LZMA static char * load_module_file_lzma(const char *filename, size_t *len) { @@ -548,6 +551,15 @@ load_module_file_lzma(const char *filename, size_t *len) free(xzdata); return data; } +#else +static char * +load_module_file_lzma(const char *filename, size_t *len) +{ +fprintf(stderr, libvirt-sandbox-init-qemu: %s: +lzma support disabled, can't read module %s\n, __func__, filename); +exit_poweroff(); +} +#endif /* WITH_LZMA */ static char * load_module_file_zlib(const char *filename, size_t *len) -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox 02/10] Allow disabling zlib support.
On Thu, Jun 25, 2015 at 06:49:39PM +0200, Cédric Bosdonnat wrote: Some distributions may not have static zlib package. Allow disabling it at build time. --- configure.ac| 16 +++- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 12 2 files changed, 27 insertions(+), 1 deletion(-) ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox 03/10] Enable strcmp checks in libvirt-sandbox-init-qemu.c
On Thu, Jun 25, 2015 at 06:49:40PM +0200, Cédric Bosdonnat wrote: --- cfg.mk | 2 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) ACK, but... diff --git a/cfg.mk b/cfg.mk index 83ded15..e9c12f7 100644 --- a/cfg.mk +++ b/cfg.mk @@ -133,4 +133,4 @@ exclude_file_name_regexp--sc_bindtextdomain = ^(libvirt-sandbox/tests)|(libvirt- exclude_file_name_regexp--sc_preprocessor_indentation = ^*/*.[ch] -exclude_file_name_regexp--sc_prohibit_strcmp = ^libvirt-sandbox/libvirt-sandbox-init-qemu.c +#exclude_file_name_regexp--sc_prohibit_strcmp = ^libvirt-sandbox/libvirt-sandbox-init-qemu.c You could just delete this instead of commenting it out Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 04/10] Add disk support to the container builder
On Thu, Jun 25, 2015 at 01:27:24PM +0200, Eren Yagdiran wrote: Use the new disk configuration in the container builder to provide disks in lxc containers sandboxes. --- .../libvirt-sandbox-builder-container.c| 37 +- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c index 59bfee1..3318c30 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c @@ -218,14 +218,49 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil GVirConfigDomainInterfaceNetwork *iface; GVirConfigDomainConsole *con; GVirConfigDomainChardevSourcePty *src; -GList *tmp = NULL, *mounts = NULL, *networks = NULL; +GVirConfigDomainDisk *disk; +GVirConfigDomainDiskDriver *diskDriver; +GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL; gchar *configdir = g_strdup_printf(%s/config, statedir); gboolean ret = FALSE; +size_t nVirtioDev = 0; if (!GVIR_SANDBOX_BUILDER_CLASS(gvir_sandbox_builder_container_parent_class)- construct_devices(builder, config, statedir, domain, error)) goto cleanup; + +tmp = disks = gvir_sandbox_config_get_disks(config); +while(tmp){ +GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp-data); + +if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){ + + gchar *device = g_strdup_printf(vd%c, (char)('a' + nVirtioDev++)); Disk names are utterly arbitrary in LXC, but I think I'd suggest using 'sda' rather than 'vda', since the latter suggests use of virtio, where as sda is a generic naming scheme. +disk = gvir_config_domain_disk_new(); +diskDriver = gvir_config_domain_disk_driver_new(); +gvir_config_domain_disk_set_type(disk, + gvir_sandbox_config_disk_get_disk_type(dconfig)); +gvir_config_domain_disk_driver_set_format(diskDriver, + gvir_sandbox_config_disk_get_format(dconfig)); +gvir_config_domain_disk_driver_set_name(diskDriver, nbd); We can probably just leave out nbd and let libvirt do the right thing to pick the driver based on the declared image format. ie there's no point in forcing nbd when the format is raw. +gvir_config_domain_disk_set_source(disk, + gvir_sandbox_config_disk_get_source(dconfig)); +gvir_config_domain_disk_set_target_bus(disk, + GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO); Don't set bus at all for containers - its irrelevant and (currently) ignored. We don't want suprises if libvirt later checks the bus and rejects it for LXC. +gvir_config_domain_disk_set_target_dev(disk,device); +gvir_config_domain_disk_set_driver(disk, diskDriver); +gvir_config_domain_add_device(domain, + GVIR_CONFIG_DOMAIN_DEVICE(disk)); +g_object_unref(disk); +} +tmp = tmp-next; +} + +g_list_foreach(disks, (GFunc)g_object_unref, NULL); +g_list_free(disks); + Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 02/10] Add configuration object for disk support
On Thu, Jun 25, 2015 at 01:27:22PM +0200, Eren Yagdiran wrote: Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2 --- libvirt-sandbox/Makefile.am | 2 + libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 +++ libvirt-sandbox/libvirt-sandbox-config.c | 293 ++ libvirt-sandbox/libvirt-sandbox-config.h | 10 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 4 + libvirt-sandbox/tests/test-config.c | 11 + 8 files changed, 677 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h Main comment is to replace 'target' with 'tag' throughout. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] qemu: monitor: Open-code retrieval of wr_highest_offset
On 06/23/2015 01:15 PM, Peter Krempa wrote: Instead of using qemuMonitorJSONDevGetBlockExtent (which I plan to remove later) extract the data in place. Additionally add a flag that will be set when the wr_highest_offset was extracted correctly so that callers can act according to that. The test case addition should help make sure that everything works. --- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 10 -- tests/qemumonitorjsontest.c | 20 +++- tests/qemumonitortest.c | 10 +- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 1afc344..b4d6538 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -379,6 +379,7 @@ struct _qemuBlockStats { unsigned long long capacity; unsigned long long physical; unsigned long long wr_highest_offset; +bool wr_highest_offset_valid; }; Maybe some sort of comment that starting at wr_highest_offset, all values require both the data and a bool of the same name, but with _valid ... (see below - perhaps it'll make more sense) int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 11c45a1..b2ce33f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1699,6 +1699,8 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, { qemuBlockStatsPtr bstats = NULL; virJSONValuePtr stats; +virJSONValuePtr parent; +virJSONValuePtr parentstats; int ret = -1; int nstats = 0; char *entry_name = qemuDomainStorageAlias(dev_name, depth); @@ -1735,8 +1737,12 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, QEMU_MONITOR_BLOCK_STAT_GET(flush_total_time_ns, bstats-flush_total_times, false); #undef QEMU_MONITOR_BLOCK_STAT_GET -/* it's ok to not have this information here. Just skip silently. */ -qemuMonitorJSONDevGetBlockExtent(dev, bstats-wr_highest_offset); +if ((parent = virJSONValueObjectGetObject(dev, parent)) +(parentstats = virJSONValueObjectGetObject(parent, stats))) { +if (virJSONValueObjectGetNumberUlong(parentstats, wr_highest_offset, + bstats-wr_highest_offset) == 0) +bstats-wr_highest_offset_valid = true; +} if (virHashAddEntry(hash, entry_name, bstats) 0) goto cleanup; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0623275..6246737 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1546,8 +1546,16 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data) goto cleanup; \ } +#define CHECK0ULL(var, value) \ +if (stats-var != value) { \ +virReportError(VIR_ERR_INTERNAL_ERROR, \ + Invalid #var value: %llu, expected %llu, \ + stats-var, value); \ +goto cleanup; \ +} + #define CHECK(NAME, RD_REQ, RD_BYTES, RD_TOTAL_TIMES, WR_REQ, WR_BYTES, \ - WR_TOTAL_TIMES, FLUSH_REQ, FLUSH_TOTAL_TIMES) \ + WR_TOTAL_TIMES, FLUSH_REQ, FLUSH_TOTAL_TIMES, WR_HIGHEST_OFFSET) \ if (!(stats = virHashLookup(blockstats, NAME))) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ block stats for device '%s' is missing, NAME); \ @@ -1560,7 +1568,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data) CHECK0(wr_bytes, WR_BYTES) \ CHECK0(wr_total_times, WR_TOTAL_TIMES) \ CHECK0(flush_req, FLUSH_REQ) \ -CHECK0(flush_total_times, FLUSH_TOTAL_TIMES) +CHECK0(flush_total_times, FLUSH_TOTAL_TIMES) \ +CHECK0ULL(wr_highest_offset, WR_HIGHEST_OFFSET) if (qemuMonitorGetAllBlockStatsInfo(qemuMonitorTestGetMonitor(test), blockstats, false) 0) @@ -1572,9 +1581,9 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data) goto cleanup; } -CHECK(virtio-disk0, 1279, 28505088, 640616474, 174, 2845696, 530699221, 0, 0) -CHECK(virtio-disk1, 85, 348160, 8232156, 0, 0, 0, 0, 0) -CHECK(ide0-1-0, 16, 49250, 1004952, 0, 0, 0, 0, 0) +CHECK(virtio-disk0, 1279, 28505088, 640616474, 174, 2845696, 530699221, 0, 0, 5256018944ULL) +CHECK(virtio-disk1, 85, 348160, 8232156, 0, 0, 0, 0, 0, 0ULL) +CHECK(ide0-1-0, 16, 49250, 1004952, 0, 0, 0, 0, 0, 0ULL) This assumes that the wr_highest_offset was found, what about when it's not found in the data stream? Seems that would mean either modifying the CHECK macro to add another parameter (true/false) or some sort of magic in the CHECK0LL macro that does something like stats-#var_valid (I never get the exact
Re: [libvirt] [PATCH 4/5] qemu: Refactor qemuDomainGetBlockInfo
On 06/23/2015 01:15 PM, Peter Krempa wrote: Change the code so that it queries the monitor when the VM is alive. --- src/qemu/qemu_driver.c | 90 +- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 004da7e..3da941e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11800,10 +11800,12 @@ qemuDomainGetBlockInfo(virDomainPtr dom, virQEMUDriverPtr driver = dom-conn-privateData; virDomainObjPtr vm; int ret = -1; -virDomainDiskDefPtr disk = NULL; -virStorageSourcePtr src; -bool activeFail = false; +virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = NULL; +int rc; +virHashTablePtr stats = NULL; +qemuBlockStats *entry; +char *alias; virCheckFlags(0, -1); @@ -11815,11 +11817,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, if (virDomainGetBlockInfoEnsureACL(dom-conn, vm-def) 0) goto cleanup; -/* Technically, we only need a job if we are going to query the - * monitor, which is only for active domains that are using - * non-raw block devices. But it is easier to share code if we - * always grab a job; furthermore, grabbing the job ensures that - * hot-plug won't change disk behind our backs. */ Was the comment wrong? I tend to like comments like this, since it gives me an understanding why something was done a particular way... I'm not going to stop a change for removing the comment though... ACK John if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) 0) goto cleanup; @@ -11829,53 +11826,72 @@ qemuDomainGetBlockInfo(virDomainPtr dom, goto endjob; } -src = disk-src; -if (virStorageSourceIsEmpty(src)) { +if (virStorageSourceIsEmpty(disk-src)) { virReportError(VIR_ERR_INVALID_ARG, _(disk '%s' does not currently have a source assigned), path); goto endjob; } -if ((ret = qemuStorageLimitsRefresh(driver, cfg, vm, src)) 0) +/* for inactive domains we have to peek into the files */ +if (!virDomainObjIsActive(vm)) { +if ((qemuStorageLimitsRefresh(driver, cfg, vm, disk-src)) 0) +goto endjob; + +info-capacity = disk-src-capacity; +info-allocation = disk-src-allocation; +info-physical = disk-src-physical; + +ret = 0; +goto endjob; +} + +if (!disk-info.alias || +!(alias = qemuDomainStorageAlias(disk-info.alias, 0))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(missing disk device alias name for %s), disk-dst); goto endjob; +} -if (!src-allocation) { -qemuDomainObjPrivatePtr priv = vm-privateData; +qemuDomainObjEnterMonitor(driver, vm); +rc = qemuMonitorGetAllBlockStatsInfo(qemuDomainGetMonitor(vm), + stats, false); +if (rc = 0) +rc = qemuMonitorBlockStatsUpdateCapacity(qemuDomainGetMonitor(vm), + stats, false); -/* If the guest is not running, then success/failure return - * depends on whether domain is persistent - */ -if (!virDomainObjIsActive(vm)) { -activeFail = true; +if (qemuDomainObjExitMonitor(driver, vm) 0 || rc 0) +goto endjob; + +if (!(entry = virHashLookup(stats, alias))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(failed to gather stats for disk '%s'), disk-dst); +goto endjob; +} + +if (!entry-wr_highest_offset_valid) { +if (virStorageSourceGetActualType(disk-src) == VIR_STORAGE_TYPE_BLOCK +disk-src-format != VIR_STORAGE_FILE_RAW) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(failed to query the maximum written offset of + block device '%s'), disk-dst); goto endjob; } -qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorGetBlockExtent(priv-mon, -disk-info.alias, -src-allocation); -if (qemuDomainObjExitMonitor(driver, vm) 0) -ret = -1; +info-allocation = entry-physical; +} else { +info-allocation = entry-wr_highest_offset; } -if (ret == 0) { -info-capacity = src-capacity; -info-allocation = src-allocation; -info-physical = src-physical; -} +info-capacity = entry-capacity; +info-physical = entry-physical; + +ret = 0; endjob: qemuDomainObjEndJob(driver, vm); cleanup: -/* If we failed to get data from a domain because it's inactive and - * it's not a
[libvirt] [PATCH 1/2] qemuBuildMemoryBackendStr: Fix hugepages lookup process
https://bugzilla.redhat.com/show_bug.cgi?id=1196644 So this function is called to construct the guest NUMA part of the qemu command line. At the beginning, the configured hugepages are searched to find the best match for given guest NUMA node. Configured hugepages can have a @nodeset attribute to specify on which guest NUMA nodes should be the hugepages backing used. There is, however, one 'corner case'. Users may just tell 'use hugepages to back all the nodes'. In other words: memoryBacking hugepages/ /memoryBacking cpu numa cell id='0' cpus='0-1' memory='1024000' unit='KiB'/ /numa /cpu Our code fails in this case. Well, since there's no @nodeset (nor any page/ child element to hugepages/) we fail to lookup the default hugepage size to use. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_command.c| 41 .../qemuxml2argv-hugepages-numa.args | 46 + .../qemuxml2argv-hugepages-numa.xml| 107 + tests/qemuxml2argvtest.c | 8 ++ 4 files changed, 183 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 99755f1..cb31fac 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4760,9 +4760,6 @@ qemuBuildMemoryBackendStr(unsigned long long size, hugepage = master_hugepage; } -if (hugepage) -pagesize = hugepage-size; - if (hugepage hugepage-size == system_page_size) { /* However, if user specified to use huge page * of regular system page size, it's as if they @@ -4778,23 +4775,29 @@ qemuBuildMemoryBackendStr(unsigned long long size, goto cleanup; } -/* Now lets see, if the huge page we want to use is even mounted - * and ready to use */ -for (i = 0; i cfg-nhugetlbfs; i++) { -if (cfg-hugetlbfs[i].size == hugepage-size) -break; -} - -if (i == cfg-nhugetlbfs) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Unable to find any usable hugetlbfs mount for %llu KiB), - pagesize); -goto cleanup; -} - VIR_FREE(mem_path); -if (!(mem_path = qemuGetHugepagePath(cfg-hugetlbfs[i]))) -goto cleanup; +if (hugepage-size) { +/* Now lets see, if the huge page we want to use is even mounted + * and ready to use */ +for (i = 0; i cfg-nhugetlbfs; i++) { +if (cfg-hugetlbfs[i].size == hugepage-size) +break; +} + +if (i == cfg-nhugetlbfs) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Unable to find any usable hugetlbfs mount for %llu KiB), + hugepage-size); +goto cleanup; +} + +if (!(mem_path = qemuGetHugepagePath(cfg-hugetlbfs[i]))) +goto cleanup; +} else { +if (!(mem_path = qemuGetDefaultHugepath(cfg-hugetlbfs, +cfg-nhugetlbfs))) +goto cleanup; +} *backendType = memory-backend-file; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args new file mode 100644 index 000..c47e097 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args @@ -0,0 +1,46 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu-system-x86_64 \ +-S \ +-M pc-i440fx-2.3 \ +-cpu Haswell \ +-m 1024 \ +-smp 2 \ +-object memory-backend-file,id=ram-node0,prealloc=yes,\ +mem-path=/dev/hugepages2M/libvirt/qemu,size=1073741824 \ +-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-rtc base=utc,driftfix=slew \ +-no-kvm-pit-reinjection \ +-global PIIX4_PM.disable_s3=1 \ +-global PIIX4_PM.disable_s4=1 \ +-boot c \ +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x6.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x6 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x6.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x6.0x2 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 \ +-drive file=/var/lib/libvirt/images/fedora.qcow2,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0 \ +-drive if=none,media=cdrom,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-serial pty \
[libvirt] [PATCH 2/2] qemuBuildMemoryBackendStr: Honour passed @pagesize
So far the argument has not much meaning and was practically ignored. This is not good since when doing memory hotplug, the size of desired hugepage backing is passed in that argument. Taking closer look at the tests I'm fixing reveals the bug. For instance, while the following is in the test: memory model='dimm' source nodemask1-3/nodemask pagesize unit='KiB'4096/pagesize /source target size unit='KiB'524287/size node0/node /target address type='dimm' slot='0' base='0x1'/ /memory the generated commandline corresponding to this XML was: -object memory-backend-ram,id=memdimm0,size=536870912,\ host-nodes=1-3,policy=bind Have you noticed? Yes, memory-backend-ram! Nothing can be further away from the right answer. The hugepage backing is requested in the XML and we happily ignore it. This is just not right. It's memory-backend-file which should have been used: -object memory-backend-file,id=memdimm0,prealloc=yes,\ mem-path=/dev/hugepages4M/libvirt/qemu,size=536870912,\ host-nodes=1-3,policy=bind The problem is, that @pagesize passed to qemuBuildMemoryBackendStr (where this part of commandline is built) was ignored. The hugepage to back memory was searched only and only by NUMA nodes pinning. This works only for regular guest NUMA nodes. Then, I'm changing the hugepages size in the test XMLs too. This is simply because in the test suite we create dummy mount points just for 2M and 1G hugepages. And in the test 4M was requested. I'm sticking to 2M, but 1G should just work too. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_command.c| 28 -- .../qemuxml2argv-hugepages-numa.args | 5 +++- .../qemuxml2argv-hugepages-numa.xml| 11 + .../qemuxml2argv-memory-hotplug-dimm-addr.args | 4 ++-- .../qemuxml2argv-memory-hotplug-dimm-addr.xml | 2 +- .../qemuxml2argv-memory-hotplug-dimm.args | 4 ++-- .../qemuxml2argv-memory-hotplug-dimm.xml | 2 +- tests/qemuxml2argvtest.c | 8 --- 8 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cb31fac..e0214d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4727,7 +4727,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, virDomainNumatuneGetMode(def-numa, -1, mode) 0) mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; -if (pagesize == 0 || pagesize != system_page_size) { +if (pagesize == 0) { /* Find the huge page size we want to use */ for (i = 0; i def-mem.nhugepages; i++) { bool thisHugepage = false; @@ -4760,15 +4760,19 @@ qemuBuildMemoryBackendStr(unsigned long long size, hugepage = master_hugepage; } -if (hugepage hugepage-size == system_page_size) { -/* However, if user specified to use huge page - * of regular system page size, it's as if they - * hasn't specified any huge pages at all. */ -hugepage = NULL; -} +if (hugepage) +pagesize = hugepage-size; } -if (hugepage) { +if (pagesize == system_page_size) { +/* However, if user specified to use huge page + * of regular system page size, it's as if they + * hasn't specified any huge pages at all. */ +pagesize = 0; +hugepage = NULL; +} + +if (pagesize || hugepage) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(this qemu doesn't support hugepage memory backing)); @@ -4776,18 +4780,18 @@ qemuBuildMemoryBackendStr(unsigned long long size, } VIR_FREE(mem_path); -if (hugepage-size) { +if (pagesize) { /* Now lets see, if the huge page we want to use is even mounted * and ready to use */ for (i = 0; i cfg-nhugetlbfs; i++) { -if (cfg-hugetlbfs[i].size == hugepage-size) +if (cfg-hugetlbfs[i].size == pagesize) break; } if (i == cfg-nhugetlbfs) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Unable to find any usable hugetlbfs mount for %llu KiB), - hugepage-size); + pagesize); goto cleanup; } @@ -4854,7 +4858,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, goto cleanup; } -if (!hugepage) { +if (!hugepage !pagesize) { bool nodeSpecified = virDomainNumatuneNodeSpecified(def-numa, guestNode); if ((userNodeset || nodeSpecified || force) diff --git
[libvirt] [PATCH 0/2] qemuBuildMemoryBackendStr: Couple of fixes
The first one fixes a bug we have for a long time. The second one is not that ancient but still worth fixing. Michal Privoznik (2): qemuBuildMemoryBackendStr: Fix hugepages lookup process qemuBuildMemoryBackendStr: Honour passed @pagesize src/qemu/qemu_command.c| 57 +- .../qemuxml2argv-hugepages-numa.args | 49 + .../qemuxml2argv-hugepages-numa.xml| 118 + .../qemuxml2argv-memory-hotplug-dimm-addr.args | 4 +- .../qemuxml2argv-memory-hotplug-dimm-addr.xml | 2 +- .../qemuxml2argv-memory-hotplug-dimm.args | 4 +- .../qemuxml2argv-memory-hotplug-dimm.xml | 2 +- tests/qemuxml2argvtest.c | 14 ++- 8 files changed, 217 insertions(+), 33 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.xml -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 07/10] qemu: use devtmpfs rather than tmpfs to auto-populate /dev
On Thu, Jun 25, 2015 at 01:27:27PM +0200, Eren Yagdiran wrote: From: Cédric Bosdonnat cbosdon...@suse.com When using devtmpfs we don't need to care about the device nodes creation: it's less risk to forget some. It also eases the creation of the devices in the init-qemu. --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 94 + 1 file changed, 1 insertion(+), 93 deletions(-) ACK, that certainly simplifies life :-) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: monitor: Fix indentation in qemuMonitorJSONGetOneBlockStatsInfo
On 06/23/2015 01:15 PM, Peter Krempa wrote: --- src/qemu/qemu_monitor_json.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 08/10] Init-util : Common directory functions for init-common and init-qemu
On Thu, Jun 25, 2015 at 01:27:28PM +0200, Eren Yagdiran wrote: Init-util provides common functions for creating directories for both Init-common and Init-qemu. Error handling needs to be done in calling function. --- libvirt-sandbox/Makefile.am | 4 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 57 +--- libvirt-sandbox/libvirt-sandbox-init-util.c | 58 + libvirt-sandbox/libvirt-sandbox-init-util.h | 41 4 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h Creating a shared file for these two functions is not really required. They are needed in init-qemu because that file only relies on POSIX APIs. In init-common we have the ability to use glib, so we you can just use +int gvir_sandbox_init_util_mkdir(const char *dir, int mode, int debug); g_mkdir_with_parents(dir) +int gvir_sandbox_init_util_mkparent(const char *dir, int mode, int debug); parent = g_path_get_dirname(dir) g_mkdir_with_parents(parent) g_free(parent) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] vz: add memory statistics
On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote: From: Nikolay Shirokovskiy nshirokovs...@parallels.com Implemented counters: VIR_DOMAIN_MEMORY_STAT_SWAP_IN VIR_DOMAIN_MEMORY_STAT_SWAP_OUT VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT VIR_DOMAIN_MEMORY_STAT_AVAILABLE VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON VIR_DOMAIN_MEMORY_STAT_UNUSED Comments. 1. Use vzDomObjFromDomainRef/virDomainObjEndAPI pair to get domain object as we use prlsdkGetStatsParam. See previous statistics comments. 2. Balloon statistics is not applicable to containers. Fault statistics for containers not provided in PCS6 yet. At some reason it returns -1 for all counters on my server, need to check, why is it... Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/vz/vz_driver.c | 73 1 files changed, 73 insertions(+), 0 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 4197569..8dae7c4 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1377,6 +1377,78 @@ vzDomainInterfaceStats(virDomainPtr domain, return ret; } +static int +vzDomainMemoryStats(virDomainPtr domain, + virDomainMemoryStatPtr stats, + unsigned int nr_stats, + unsigned int flags) +{ +virDomainObjPtr dom = NULL; +int ret = -1; +long long v = 0, t = 0, u = 0; +size_t i = 0; + +virCheckFlags(0, -1); +if (!(dom = vzDomObjFromDomainRef(domain))) +return -1; + +#define PARALLELS_GET_COUNTER(NAME, VALUE) \ +if (prlsdkGetStatsParam(dom, NAME, VALUE) 0) \ +goto cleanup; \ + +#define PARALLELS_MEMORY_STAT_SET(TAG, VALUE) \ +if (i nr_stats) { \ +stats[i].tag = (TAG); \ +stats[i].val = (VALUE); \ +i++;\ +} + +#define PARALLELS_COUNTER_PROTECT(VALUE) VALUE == -1 : ? + +i = 0; + +// count to kb +PARALLELS_GET_COUNTER(guest.ram.swap_in, v) +if (v != -1) +PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_SWAP_IN, v 12) + +PARALLELS_GET_COUNTER(guest.ram.swap_out, v) +if (v != -1) +PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, v 12) + +PARALLELS_GET_COUNTER(guest.ram.minor_fault, v) +if (v != -1) +PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, v) + +PARALLELS_GET_COUNTER(guest.ram.major_fault, v) +if (v != -1) +PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, v) To be honest, I don't think macros here improve code readability, look, how it would be without them: if (prlsdkGetStatsParam(dom, guest.ram.major_fault, v) 0) goto cleanup; if (v != -1 i nr_stats) { stats[i].tag = VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT; stats[i].val = v; i++; } Only goto cleanup and i++ are repeating information. + +PARALLELS_GET_COUNTER(guest.ram.total, v) +if (v != -1) +PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, v 10) + +PARALLELS_GET_COUNTER(guest.ram.balloon_actual, v) +if (v != -1) +PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, v 10) + +PARALLELS_GET_COUNTER(guest.ram.usage, u) +PARALLELS_GET_COUNTER(guest.ram.total, t) +if (u != -1 t != -1) +PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_UNUSED, (t - u) 10) + + +#undef PARALLELS_GET_COUNTER +#undef PARALLELS_MEMORY_STAT_SET + +ret = i; + cleanup: +if (dom) +virDomainObjEndAPI(dom); + +return ret; +} + static virHypervisorDriver vzDriver = { .name = vz, .connectOpen = vzConnectOpen,/* 0.10.0 */ @@ -1429,6 +1501,7 @@ static virHypervisorDriver vzDriver = { .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */ .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */ .domainInterfaceStats = vzDomainInterfaceStats, /* 1.3.0 */ +.domainMemoryStats = vzDomainMemoryStats, /* 1.3.0 */ }; static virConnectDriver vzConnectDriver = { -- Dmitry Guryanov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] qemu: Redesigning guest CPU configuration
On Wed, 17 Jun 2015 17:37:42 +0200 Jiri Denemark jdene...@redhat.com wrote: Hi all (and sorry for the long email), The current way QEMU driver handles guest CPU configuration is not ideal. We detect host CPU capabilities only by querying the CPU and we don't check with QEMU what features it supports. We don't check QEMU's definitions of CPU models, which may be different from libvirt's definitions. All this results in several issues: - guest CPU may change druing migration, save/restore - libvirt may ask for a CPU which QEMU cannot provide; the guest will see a slightly different CPU but libvirt client won't know about it - libvirt may come up with a CPU that doesn't make sense and which won't work for a guest (the guest may even crash) Although usually everything just works, it is very fragile. Since we want to fix all these issues, we need to: - guarantee stable guest ABI (a single domain XML should always results in the same guest ABI). Once a domain is started, its CPU definition should never change (unless someone changes the XML, of course, similar to, e.g. PCI addresses). However, there are a few exceptions: - host-passthrough CPU mode will always result in -cpu host - host-model CPU mode should recompute the CPU model on every start, but the CPU must not change during migration - always make sure QEMU provides the CPU we asked for. Starting a domain should fail in case QEMU cannot provide exactly the CPU we asked for. - provide usable host-model mode and custom mode with minimum match. We need to generate CPU configurations that actually work, i.e., we need to ask QEMU what CPU it can provide on current host rather than requesting a bunch of features on top of a CPU model which does not always match the host CPU. QEMU already provides or will soon provide everything we need to meet these requirements: - we can cover every configurable part of a CPU in our cpu_map.xml and instead of asking QEMU for a specific CPU model we can use -cpu custom with a fully specified CPU - we can use the additional data about CPU models to choose the right one for a host CPU - when starting a domain we can check whether QEMU filtered out any of the features we asked for and refuse to start the domain - we can ask QEMU what would -cpu host look like and use that for host-model and minimum match CPUs (it won't work for TCG mode, though, but we can keep using the current CPUID detection code for TCG) Once we start maintaining CPU models with all the details, we will likely meet the same issues QEMU folks meet, i.e., we will need to fix bugs in existing CPU models. And it's not just about adding removing CPU features but also fixing other parameters, such as wrong level, etc. It's clear every change will require a new CPU model to be defined. But I think we should do it in a way that applications or users should not need (if they don't want to) to care about it. I'm thinking about doing something similar to machine types. Each CPU model could be defined in several versions and a CPU specs without a version would be an alias to the latest version. The problem is, we need to maintain backward compatibility and we should avoid breaking existing domains (shouldn't we?) which just work even though their guest CPUs do not exactly match the domain XML definitions. So either we need to define all existing CPU models in all their variants used for various machine types and have a mapping between (model without a version, machine type) to a specific version of the model (which may be quite hard) or we need to be able to distinguish between an existing domain and a new domain with no CPU model version. While host-model and host-passthrough CPU modes are easy because they are designed to change everytime a domain starts (which means we don't need to be able to distinguish between existing and new domains), custom CPU mode are tricky. Currently, the only at least a bit reasonable thing which came to my mind is to have a new CPU mode, but it still seems awkward so please share your ideas if you have any. BTW, I don't think we should try to expose every part of the CPU model definitions in domain XML, they should remain hidden behind the CPU model name. It would be hard to explain what each of the extra parameters mean, each model would have to include them anyway since we can't expect users to provide all the details correctly, and once visible in domain XML it could encourage users to play with the values. I'm looking forward to any comments or ideas. Hi Jiri, I basically agree with your analysis on the current situation but are in doubt that the below sketched proposal will overcome the issue. Beside the CPU model and its features also the QEMU level (-machine version), the accellerator type (-accel KVM, TCG, ...) also the kernel (KVM) and the host CPU will determine which CPU capabilities will be
[libvirt] [sandbox 02/10] Allow disabling zlib support.
Some distributions may not have static zlib package. Allow disabling it at build time. --- configure.ac| 16 +++- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 12 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 834a444..99d22d7 100644 --- a/configure.ac +++ b/configure.ac @@ -80,7 +80,16 @@ PKG_CHECK_MODULES(LIBVIRT, libvirt = $LIBVIRT_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GLIB, libvirt-glib-1.0 = $LIBVIRT_GOBJECT_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GOBJECT, libvirt-gobject-1.0 = $LIBVIRT_GOBJECT_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GCONFIG, libvirt-gconfig-1.0 = $LIBVIRT_GCONFIG_REQUIRED) -PKG_CHECK_MODULES(ZLIB, zlib = $ZLIB_REQUIRED) + +AC_ARG_WITH([zlib], + [AS_HELP_STRING([--with-zlib], +[add ZLIB support @:@default=yes@:@])]) +m4_divert_text([DEFAULTS], [with_zlib=yes]) + +if test $with_zlib = yes ; then +PKG_CHECK_MODULES(ZLIB, zlib = $ZLIB_REQUIRED) +fi + AC_ARG_WITH([lzma], [AS_HELP_STRING([--with-lzma], [add LZMA support @:@default=yes@:@])]) @@ -130,6 +139,11 @@ AC_MSG_NOTICE([LZMA: $LZMA_CFLAGS $LZMA_LIBS]) else AC_MSG_NOTICE([LZMA: no]) fi +if test $with_zlib != no ; then +AC_MSG_NOTICE([ZLIB: $ZLIB_CFLAGS $ZLIB_LIBS]) +else +AC_MSG_NOTICE([ZLIB: no]) +fi AC_MSG_NOTICE([ GOBJECT: $GOBJECT_CFLAGS $GOBJECT_LIBS]) AC_MSG_NOTICE([ LIBVIRT_GOBJECT: $LIBVIRT_GOBJECT_CFLAGS $LIBVIRT_GOBJECT_LIBS]) AC_MSG_NOTICE([]) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index e91dbcf..a20db77 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -45,7 +45,9 @@ #if WITH_LZMA #include lzma.h #endif /* WITH_LZMA */ +#if WITH_ZLIB #include zlib.h +#endif /* WITH_ZLIB */ #define ATTR_UNUSED __attribute__((__unused__)) @@ -561,6 +563,7 @@ load_module_file_lzma(const char *filename, size_t *len) } #endif /* WITH_LZMA */ +#if WITH_ZLIB static char * load_module_file_zlib(const char *filename, size_t *len) { @@ -611,6 +614,15 @@ load_module_file_zlib(const char *filename, size_t *len) gzclose(fp); return data; } +#else +static char * +load_module_file_zlib(const char *filename, size_t *len) +{ +fprintf(stderr, libvirt-sandbox-init-qemu: %s: +zlib support disabled, can't read module %s\n, __func__, filename); +exit_poweroff(); +} +#endif /* WITH_ZLIB */ static char * load_module_file_raw(const char *filename, size_t *len) -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 06/10] init-qemu: extract the mounts.cfg entry mounting code
Create a mount_entry function from the code mounting the entries defined in mounts.cfg in order to be able to reuse that code. This will later be useful to mount a / from mounts.cfg. --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 68 + 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 45cb9b3..9acea5f 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -226,6 +226,44 @@ create_virtioblk_device(const char *dev) } } +static void +mount_entry(const char *source, +const char *target, +const char *type, +const char * opts) +{ +int flags = 0; + +if (strncmp(source, /dev/vd, 7) == 0) +create_virtioblk_device(source); + +if (STREQ(type, )) { +struct stat st; +type = NULL; +flags |= MS_BIND; +if (stat(source, st) 0) { +fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot read mount source %s: %s\n, +__func__, source, strerror(errno)); +exit_poweroff(); +} +if (S_ISDIR(st.st_mode)) +mount_mkdir(target, 755); +else +mount_mkfile(target, 644); +} else { +if (STREQ(type, tmpfs)) +flags |= MS_NOSUID | MS_NODEV; + +mount_mkdir(target, 0755); +} + +if (mount(source, target, type, flags, opts) 0) { +fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot mount %s on %s (%s, %s): %s\n, +__func__, source, target, type, opts, strerror(errno)); +exit_poweroff(); +} +} + int main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) { @@ -369,40 +407,12 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) opts++; char *tmp = strchr(opts, '\n'); *tmp = '\0'; -int flags = 0; if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s - %s (%s, %s)\n, __func__, source, target, type, opts); -if (strncmp(source, /dev/vd, 7) == 0) -create_virtioblk_device(source); - -if (STREQ(type, )) { -struct stat st; -type = NULL; -flags |= MS_BIND; -if (stat(source, st) 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot read mount source %s: %s\n, -__func__, source, strerror(errno)); -exit_poweroff(); -} -if (S_ISDIR(st.st_mode)) -mount_mkdir(target, 755); -else -mount_mkfile(target, 644); -} else { -if (STREQ(type, tmpfs)) -flags |= MS_NOSUID | MS_NODEV; - -mount_mkdir(target, 0755); -} - -if (mount(source, target, type, flags, opts) 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot mount %s on %s (%s, %s): %s\n, -__func__, source, target, type, opts, strerror(errno)); -exit_poweroff(); -} +mount_entry(source, target, type, opts); } fclose(fp); -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 07/10] qemu: use mounts targeting / as root
So far a mount with / as target doesn't change anything: the host / is still the one mounted as /. libvirt-sandbox-init-qemu now detects the presence of a / target in mounts.cfg and mounts it instead of sandbox:root. --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 69 - 1 file changed, 67 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 9acea5f..02fb980 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -264,6 +264,70 @@ mount_entry(const char *source, } } +static void +mount_root(const char *path) +{ +int foundRoot = 0; + +/* Loop over mounts.cfg to see if we have a candidate for / */ +mount_mkdir(SANDBOXCONFIGDIR, 0755); +mount_9pfs(sandbox:config, SANDBOXCONFIGDIR, 0755, 1); + +FILE *fp = fopen(SANDBOXCONFIGDIR /mounts.cfg, r); +while (fgets(line, sizeof line, fp) !foundRoot) { +char *source = line; +char *target = strchr(source, '\t'); +*target = '\0'; +target++; +char *type = strchr(target, '\t'); +*type = '\0'; +type++; +char *opts = strchr(type, '\t'); +*opts = '\0'; +opts++; +char *tmp = strchr(opts, '\n'); +*tmp = '\0'; + +if (STREQ(target, /)) { +int needsDev = strncmp(source, /dev/, 5) == 0; + +if (debug) +fprintf(stderr, libvirt-sandbox-init-qemu: found root from %s\n, +source); + +/* In this case, we need to have a /dev before the chroot */ +if (needsDev) { +mount_other(/proc, proc, 0755); +mount_other(/dev, tmpfs, 0755); +} + +mount_entry(source, path, type, opts); + +if (needsDev umount(/dev) 0) { +fprintf(stderr, +libvirt-sandbox-init-qemu: %s: +cannot unmount temporary /dev: %s\n, +__func__, strerror(errno)); +exit_poweroff(); +} +foundRoot = 1; +} +} +fclose(fp); + +if (umount(SANDBOXCONFIGDIR) 0) { +fprintf(stderr, +libvirt-sandbox-init-qemu: %s: +cannot unmount temporary %s: %s\n, +__func__, SANDBOXCONFIGDIR, strerror(errno)); +exit_poweroff(); +} + +/* If we couldn't get a / in the mounts, then use the host one */ +if (!foundRoot) +mount_9pfs(sandbox:root, path, 0755, 1); +} + int main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) { @@ -308,7 +372,7 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: mounting new root on /tmproot\n); -mount_9pfs(sandbox:root, /tmproot, 0755, 1); +mount_root(/tmproot); /* Note that pivot_root won't work. See the note in * Documentation/filesystems/ramfs-rootfs-initramfs.txt @@ -412,7 +476,8 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s - %s (%s, %s)\n, __func__, source, target, type, opts); -mount_entry(source, target, type, opts); +if (STREQ(target, /)) +mount_entry(source, target, type, opts); } fclose(fp); -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox 04/10] Copy init-common and all its deps to config subdir
In order to be able to mount a custom host-image as / we need to be able to access libvirt-sandbox-init-common and all its needed dependencies. They are now copied into SANDBOXCONFIGDIR /.libs. Hard linking is not possible since we may be working on separate partitions, and symlinks wouldn't help to work with apparmor. Copying makes apparmor happy and solves our problem. --- configure.ac | 7 ++ libvirt-sandbox/libvirt-sandbox-builder-machine.c | 114 ++ libvirt-sandbox/libvirt-sandbox-init-qemu.c | 5 +- 3 files changed, 124 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 99d22d7..608f56b 100644 --- a/configure.ac +++ b/configure.ac @@ -109,6 +109,13 @@ LIBVIRT_SANDBOX_SELINUX LIBVIRT_SANDBOX_STATIC_LIBC +dnl search for LDD path +AC_PATH_PROG([LDD_PATH], [ldd]) +if test -z $LDD_PATH; then +AC_MSG_ERROR([Failed to find ldd.]) +fi +AC_DEFINE_UNQUOTED([LDD_PATH], $LDD_PATH, [path to ldd binary]) + GOBJECT_INTROSPECTION_CHECK([$GOBJECT_INTROSPECTION_REQUIRED]) dnl Should be in m4/virt-gettext.m4 but intltoolize is too diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 17a2afe..7a2af83 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -358,6 +358,81 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * return ret; } +static gboolean gvir_sandbox_builder_machine_copy_lib(const char *path, + const char *libsdir, + GError **error) +{ +gchar *name = g_path_get_basename(path); +gchar *target = g_build_filename(libsdir, name, NULL); +GFile *srcFile = g_file_new_for_path(path); +GFile *tgtFile = g_file_new_for_path(target); +gboolean result = FALSE; + + +if (!g_file_copy(srcFile, tgtFile, 0, NULL, NULL, NULL, error)) +goto cleanup; + +result = TRUE; + + cleanup: +g_object_unref(tgtFile); +g_object_unref(srcFile); +g_free(target); +g_free(name); + +return result; +} + +static gboolean gvir_sandbox_builder_machine_copy_init(const gchar *statedir, + GError **error) +{ +gchar *libsdir; +const gchar *initPath = LIBEXECDIR /libvirt-sandbox-init-common; +gchar *out = NULL; +gchar *line, *tmp; +const gchar *argv[] = {LDD_PATH, initPath, NULL}; +gboolean result = FALSE; + +libsdir = g_build_filename(statedir, config, .libs, NULL); + +g_mkdir_with_parents(libsdir, 0755); + +if (!gvir_sandbox_builder_machine_copy_lib(initPath, libsdir, error)) +goto cleanup; + +/* Get all the dependencies to be hard linked */ +if (!g_spawn_sync(NULL, (gchar **)argv, NULL, 0, + NULL, NULL, out, NULL, NULL, error)) +goto cleanup; + +/* Loop over the output lines to get the path to the libraries to hard link */ +line = out; +while ((tmp = strchr(line, '\n'))) { +gchar *start, *end; +*tmp = '\0'; + +/* Search the line for the library path */ +start = strstr(line, = ); +end = strstr(line, (); + +if (start end) { +start = start + 4; +*end = '\0'; + +if (!gvir_sandbox_builder_machine_copy_lib(start, libsdir, error)) +goto cleanup; +} + +line = tmp + 1; +} +result = TRUE; + + cleanup: +g_free(libsdir); +g_free(out); + +return result; +} static gboolean gvir_sandbox_builder_machine_construct_domain(GVirSandboxBuilder *builder, GVirSandboxConfig *config, @@ -370,6 +445,9 @@ static gboolean gvir_sandbox_builder_machine_construct_domain(GVirSandboxBuilder error)) return FALSE; +if (!gvir_sandbox_builder_machine_copy_init(statedir, error)) +return FALSE; + if (!GVIR_SANDBOX_BUILDER_CLASS(gvir_sandbox_builder_machine_parent_class)- construct_domain(builder, config, statedir, domain, error)) return FALSE; @@ -712,12 +790,48 @@ static gboolean gvir_sandbox_builder_machine_clean_post_stop(GVirSandboxBuilder GError **error) { gchar *mntfile = g_strdup_printf(%s/config/mounts.cfg, statedir); +gchar *libsdir = g_build_filename(statedir, config, .libs, NULL); +GFile *libsFile = g_file_new_for_path(libsdir); +GFileEnumerator *enumerator = NULL; +GFileInfo *info = NULL; +GFile *child = NULL; gboolean ret = TRUE; if (unlink(mntfile) 0 errno != ENOENT) ret = FALSE; +if (!(enumerator = g_file_enumerate_children(libsFile, *, G_FILE_QUERY_INFO_NONE, +
Re: [libvirt] [PATCH 5/5] qemu: monitor: Remove qemuMonitorGetBlockExtent
On 06/23/2015 01:15 PM, Peter Krempa wrote: Now that qemuMonitorGetAllBlockStatsInfo collects also wr_highest_offset the whole function can be killed. --- src/qemu/qemu_monitor.c | 16 - src/qemu/qemu_monitor.h | 3 - src/qemu/qemu_monitor_json.c | 138 --- src/qemu/qemu_monitor_json.h | 3 - src/qemu/qemu_monitor_text.c | 10 src/qemu/qemu_monitor_text.h | 3 - tests/qemumonitorjsontest.c | 34 --- 7 files changed, 207 deletions(-) ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] qemu: Refactor qemuDomainGetBlockInfo
On Thu, Jun 25, 2015 at 13:26:35 -0400, John Ferlan wrote: On 06/23/2015 01:15 PM, Peter Krempa wrote: Change the code so that it queries the monitor when the VM is alive. --- src/qemu/qemu_driver.c | 90 +- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 004da7e..3da941e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11800,10 +11800,12 @@ qemuDomainGetBlockInfo(virDomainPtr dom, virQEMUDriverPtr driver = dom-conn-privateData; virDomainObjPtr vm; int ret = -1; -virDomainDiskDefPtr disk = NULL; -virStorageSourcePtr src; -bool activeFail = false; +virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = NULL; +int rc; +virHashTablePtr stats = NULL; +qemuBlockStats *entry; +char *alias; virCheckFlags(0, -1); @@ -11815,11 +11817,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, if (virDomainGetBlockInfoEnsureACL(dom-conn, vm-def) 0) goto cleanup; -/* Technically, we only need a job if we are going to query the - * monitor, which is only for active domains that are using - * non-raw block devices. But it is easier to share code if we - * always grab a job; furthermore, grabbing the job ensures that - * hot-plug won't change disk behind our backs. */ Was the comment wrong? I tend to like comments like this, since it gives me an understanding why something was done a particular way... qemuStorageLimitsRefresh modifies few fields of the disk-src structure. They are only integer modifications of capacity and they are guarded by the vm lock but that still should be covered by a job semantically. There are only very few exceptions to that rule. Additionally, grabbing the job helps to prevent the situation where we'd check that the VM is alive (where we do need a job to query the monitor), then we'd enter the job, but the vm would die at that precise moment and the rollback path from that situation would be convoluted. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_hotplug: try harder to eject media
On 06/25/2015 01:52 PM, Pavel Hrdina wrote: On Thu, Jun 25, 2015 at 12:06:49PM -0400, John Ferlan wrote: On 06/23/2015 07:13 AM, Pavel Hrdina wrote: Some guests lock the tray and QEMU eject command will simply fail to eject the media. But the guest OS can handle this attempt to eject the media and can unlock the tray and open it. In this case, we should try again to actually eject the media. If the first attempt fails we will wait for tray to open and try again to eject the media, but if the second attempt fails, returns with error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1147471 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_hotplug.c | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0628964..0cebaad 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -201,13 +201,17 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } if (ret 0) -goto error; +VIR_DEBUG(tray is probably locked, wait for the guest to unlock + the tray and open it); There are more reasons than waiting for the unlock that would cause a negative value to be returned, so this seems dangerous.. Yes, I know that. That's why the code will wait for the event DEVICE_TRAY_MOVED and only if that event where emitted from QEMU, then we will tray again. Otherwise the original error from the first attempt will be used. I didn't see DEVICE_TRAY_MOVED in this patch... The wait loop I see below was VIR_DOMAIN_DISK_TRAY_OPEN. !mon if json !cmd qemuMonitorJSONCommand failure qemuMonitorJSONCheckError failure else virAsprintf Failure qemuMonitorHMPCommand failure c_strcasestr failure Is there no way to determine that the failure was due to a locked tray from the error message? The bz notes a DEVICE_TRAY_MOVED event, but I don't see that here. Yes, there is a way, that we can parse the error message and wait only, if it contains the phrase is locked. For that QEMU event we have a handler registered and this handler updates the disk-tray_status. IOW, checking the disk-tray_status is the same as waiting for DEVICE_TRAY_MOVED event. Right and I didn't see that so it wasn't clear to me how this patch fixed the problem I would think that if this is handle-able from qemu/json only, then the qemuMonitorJSONEjectMedia would be the best place to at least check for this path of retry logic. You'll have the json error returned (per the link in bz comment 3): {error: {class: GenericError, desc: Device 'cd' is locked}} That should be parse-able by this code and either handled back in the caller or in this code. Also HMP returns the same error, so it's definitely parse-able for both monitors. At first I didn't want to go through parsing the error message, but it makes sense to retry only in case, that the error message contains is locked message. Since qemuMonitorJSONEjectMedia doesn't list return values, perhaps use -1 to mean total failure, 0 to mean pseudo-success as it does today, and 1 to indicate tray was locked and that the caller can/should retry. FWIW: By pseudo-success... I see that if 0 is currently returned, the while loop will look for the tray_status == VIR_DOMAIN_DISK_TRAY_OPEN and fail if not eventually seen Consider the following... bool retry_lock_tray = false; do { qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorEjectMedia(priv-mon, driveAlias, force); if (qemuDomainObjExitMonitor(driver, vm) 0) { ret = -1; goto cleanup; } /* We already tried, but got the locked error again */ if (ret == 1 retry_lock_tray) { some sort of failure since you retried ret = -1; goto error; } /* We were locked, but haven't retried */ if (ret == 1) { Wait some period of time for the DEVICE_TRAY_MOVED event if event found retry_lock_tray = true; } } while (retry_lock_tray); if (ret 0) goto error; if (ret == 1) { ERROR tray locked by guest, never got DEVICE_TRAY_MOVED ret = -1; got error; } I'll post a v2. virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ while (retries) { -if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) +if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { +ret = 0; +virResetLastError(); break; +} retries--; virObjectUnlock(vm); If it's locked how would this loop ever succeed? That is how would the tray open magically if it's locked and we haven't sent another eject request? I
Re: [libvirt] [PATCH] storage: add RBD support to disk source pool translation
On 06/25/2015 06:21 AM, Thibault VINCENT wrote: On 09/06/2015 17:33, John Ferlan wrote: I still think I need a tweak on what I posted as an update - the libvirtd restart path is always a bit tricky. The 'pooltype' isn't filled in at XML processing time and virStorageTranslateDiskSourcePool is only called after XML processing (sigh), so relying on it won't work. Working to figure out something generic and will repost my patch. John, I got stuck on this and would appreciate some advice to deal with the restart. With a revised patch it's still impossible to get libvirtd reconnecting to VMs using RBD source pool, because the network pool is never active so early. It goes like this : + qemuProcessReconnect ++ virStorageTranslateDiskSourcePool +++ virStoragePoolIsActive storagePoolIsActive = 0 ++ goto error ++ qemuProcessStop But seconds after it's seen as active. Isn't it a bad idea to consider a failure at translating source pool as an error in this code path? It looks quite dangerous whatever the pool protocol may be. After all the domain is still happily running anyway. Cheers Black magic - you have to go further in the call stack to what calls qemuProcessReconnect to fully understand, but along the way I'll bet storagePoolUpdateState is called My assumption without doing too much thinking is that at some point the virStorageBackendRBDRefreshPool will reconnect for you in the storageDriverAutostart path... While not exactly the same, consider how the iSCSI pool handles uses the checkPool callback. During storagePoolUpdateState, if the 'checkPool' callback determines the pool is active, then storagePoolUpdateState will call refreshPool with a NULL conn; otherwise, we wait for Autostart. So perhaps if the RBD backend had a checkPool that only did some sort of cursory check to ensure there was the infrastructure necessary for the ensuing refreshPool to work. Of course in the UpdateState path, 'conn == NULL', so the virStorageBackendRBDOpenRADOSConn won't be happy if it has authdata, but that's a decision you can make in your refreshPool callback... I don't have a conn, thus I cannot do something. HTH, John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_hotplug: try harder to eject media
On Thu, Jun 25, 2015 at 12:06:49PM -0400, John Ferlan wrote: On 06/23/2015 07:13 AM, Pavel Hrdina wrote: Some guests lock the tray and QEMU eject command will simply fail to eject the media. But the guest OS can handle this attempt to eject the media and can unlock the tray and open it. In this case, we should try again to actually eject the media. If the first attempt fails we will wait for tray to open and try again to eject the media, but if the second attempt fails, returns with error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1147471 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_hotplug.c | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0628964..0cebaad 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -201,13 +201,17 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } if (ret 0) -goto error; +VIR_DEBUG(tray is probably locked, wait for the guest to unlock + the tray and open it); There are more reasons than waiting for the unlock that would cause a negative value to be returned, so this seems dangerous.. Yes, I know that. That's why the code will wait for the event DEVICE_TRAY_MOVED and only if that event where emitted from QEMU, then we will tray again. Otherwise the original error from the first attempt will be used. !mon if json !cmd qemuMonitorJSONCommand failure qemuMonitorJSONCheckError failure else virAsprintf Failure qemuMonitorHMPCommand failure c_strcasestr failure Is there no way to determine that the failure was due to a locked tray from the error message? The bz notes a DEVICE_TRAY_MOVED event, but I don't see that here. Yes, there is a way, that we can parse the error message and wait only, if it contains the phrase is locked. For that QEMU event we have a handler registered and this handler updates the disk-tray_status. IOW, checking the disk-tray_status is the same as waiting for DEVICE_TRAY_MOVED event. I would think that if this is handle-able from qemu/json only, then the qemuMonitorJSONEjectMedia would be the best place to at least check for this path of retry logic. You'll have the json error returned (per the link in bz comment 3): {error: {class: GenericError, desc: Device 'cd' is locked}} That should be parse-able by this code and either handled back in the caller or in this code. Also HMP returns the same error, so it's definitely parse-able for both monitors. At first I didn't want to go through parsing the error message, but it makes sense to retry only in case, that the error message contains is locked message. Since qemuMonitorJSONEjectMedia doesn't list return values, perhaps use -1 to mean total failure, 0 to mean pseudo-success as it does today, and 1 to indicate tray was locked and that the caller can/should retry. FWIW: By pseudo-success... I see that if 0 is currently returned, the while loop will look for the tray_status == VIR_DOMAIN_DISK_TRAY_OPEN and fail if not eventually seen Consider the following... bool retry_lock_tray = false; do { qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorEjectMedia(priv-mon, driveAlias, force); if (qemuDomainObjExitMonitor(driver, vm) 0) { ret = -1; goto cleanup; } /* We already tried, but got the locked error again */ if (ret == 1 retry_lock_tray) { some sort of failure since you retried ret = -1; goto error; } /* We were locked, but haven't retried */ if (ret == 1) { Wait some period of time for the DEVICE_TRAY_MOVED event if event found retry_lock_tray = true; } } while (retry_lock_tray); if (ret 0) goto error; if (ret == 1) { ERROR tray locked by guest, never got DEVICE_TRAY_MOVED ret = -1; got error; } I'll post a v2. virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ while (retries) { -if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) +if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { +ret = 0; +virResetLastError(); break; +} retries--; virObjectUnlock(vm); If it's locked how would this loop ever succeed? That is how would the tray open magically if it's locked and we haven't sent another eject request? I don't understand this question?? The vm object is locked before we enter this function and this loop only unlock the vm object for the time it is waiting for the event to occur. @@ -218,10 +222,29 @@
Re: [libvirt] [PATCH] conf: fix not format priority when it is zero
On 06/26/2015 05:26 AM, Martin Kletzander wrote: On Wed, Jun 24, 2015 at 12:00:36PM +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1235116 We do not format the priority if it is 0, but this will be a broken settings in guest. Change the condition of format priority element to always format priority if scheduler is 'fifo' or 'rr'. Signed-off-by: Luyao Huang lhu...@redhat.com --- I haven't intorduce a new bool parameter to mark if we set the priority value just like the other place we avoid this issue, because i think this looks unnecessary in this place. src/conf/domain_conf.c | 6 ++-- The part for domain_conf.c didn't apply properly, but it's easy enough to fix. I modified the commit message as follows and pushed the patch: conf: Format scheduler priority when it is zero According to our XML definition, zero is as valid as any other value. Mainly because it should be kernel-agnostic. Thanks a lot for your help and quick review. Martin Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] vz: Adapt to driver rename
In the e6d180f07fb06 commit the parallels driver was renamed to vz. However, there was a commit merged later, which was sent to the list before the rename. The other commit is 6de12b026b73. Fix all the missing renames. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Pushed under build breaker rule. src/vz/vz_sdk.c | 23 --- src/vz/vz_sdk.h | 4 ++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 1a3aa87..388ea19 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2885,14 +2885,20 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, return ret; } -static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) +static int +prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) { +int ret = -1; PRL_RESULT pret; PRL_HANDLE vnet = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; -if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) -return; +if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _(unplugging network device of type %s is not supported), + virDomainNetTypeToString(net-type)); +return ret; +} pret = PrlVirtNet_Create(vnet); prlsdkCheckRetGoto(pret, cleanup); @@ -2904,16 +2910,19 @@ static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) if (PRL_FAILED(pret = waitJob(job))) goto cleanup; +ret = 0; + cleanup: PrlHandle_Free(vnet); +return ret; } int prlsdkAttachNet(virDomainObjPtr dom, -parallelsConnPtr privconn, +vzConnPtr privconn, virDomainNetDefPtr net) { int ret = -1; -parallelsDomObjPtr privdom = dom-privateData; +vzDomObjPtr privdom = dom-privateData; PRL_HANDLE job = PRL_INVALID_HANDLE; if (!IS_CT(dom-def)) { @@ -2998,11 +3007,11 @@ static int prlsdkDelNetAdapter(PRL_HANDLE sdkdom, int idx) } int prlsdkDetachNet(virDomainObjPtr dom, -parallelsConnPtr privconn, +vzConnPtr privconn, virDomainNetDefPtr net) { int ret = -1, idx = -1; -parallelsDomObjPtr privdom = dom-privateData; +vzDomObjPtr privdom = dom-privateData; PRL_HANDLE job = PRL_INVALID_HANDLE; if (!IS_CT(dom-def)) { diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index cde8904..80ff69a 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -67,6 +67,6 @@ prlsdkDetachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk); int prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr stats); int -prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net); +prlsdkAttachNet(virDomainObjPtr dom, vzConnPtr privconn, virDomainNetDefPtr net); int -prlsdkDetachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net); +prlsdkDetachNet(virDomainObjPtr dom, vzConnPtr privconn, virDomainNetDefPtr net); -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] vz_network: Reformat
Honour our formatting style. Adjust indentation so it matches the rest of our code. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/vz/vz_network.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/vz/vz_network.c b/src/vz/vz_network.c index 3a5a3ff..8c97812 100644 --- a/src/vz/vz_network.c +++ b/src/vz/vz_network.c @@ -36,9 +36,9 @@ #define VIR_FROM_THIS VIR_FROM_PARALLELS #define PARALLELS_ROUTED_NETWORK_UUID eb593dd1-6846-45b0-84a0-de0729286982 -#define vzParseError() \ -virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \ - __FUNCTION__, __LINE__, _(Can't parse prlctl output)) +#define vzParseError() \ +virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \ + __FUNCTION__, __LINE__, _(Can't parse prlctl output)) static int vzGetBridgedNetInfo(virNetworkDefPtr def, virJSONValuePtr jobj) { @@ -328,7 +328,7 @@ static int vzLoadNetworks(vzConnPtr privconn) virDrvOpenStatus vzNetworkOpen(virConnectPtr conn, - unsigned int flags) + unsigned int flags) { vzConnPtr privconn = conn-privateData; @@ -373,8 +373,8 @@ static int vzConnectNumOfNetworks(virConnectPtr conn) } static int vzConnectListNetworks(virConnectPtr conn, -char **const names, -int nnames) + char **const names, + int nnames) { vzConnPtr privconn = conn-privateData; int got; @@ -395,8 +395,8 @@ static int vzConnectNumOfDefinedNetworks(virConnectPtr conn) } static int vzConnectListDefinedNetworks(virConnectPtr conn, - char **const names, - int nnames) +char **const names, +int nnames) { vzConnPtr privconn = conn-privateData; int got; @@ -407,8 +407,8 @@ static int vzConnectListDefinedNetworks(virConnectPtr conn, } static int vzConnectListAllNetworks(virConnectPtr conn, - virNetworkPtr **nets, - unsigned int flags) +virNetworkPtr **nets, +unsigned int flags) { vzConnPtr privconn = conn-privateData; @@ -418,7 +418,7 @@ static int vzConnectListAllNetworks(virConnectPtr conn, } static virNetworkPtr vzNetworkLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) + const unsigned char *uuid) { vzConnPtr privconn = conn-privateData; virNetworkObjPtr network; @@ -439,7 +439,7 @@ static virNetworkPtr vzNetworkLookupByUUID(virConnectPtr conn, } static virNetworkPtr vzNetworkLookupByName(virConnectPtr conn, - const char *name) + const char *name) { vzConnPtr privconn = conn-privateData; virNetworkObjPtr network; @@ -460,7 +460,7 @@ static virNetworkPtr vzNetworkLookupByName(virConnectPtr conn, } static char *vzNetworkGetXMLDesc(virNetworkPtr net, -unsigned int flags) + unsigned int flags) { vzConnPtr privconn = net-conn-privateData; virNetworkObjPtr network; -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] vz_sdk: Reformat
Honour our formatting style. Adjust indentation so it matches the rest of our code. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/vz/vz_sdk.c | 209 1 file changed, 104 insertions(+), 105 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 388ea19..93427d7 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -77,7 +77,7 @@ logPrlErrorHelper(PRL_RESULT err, const char *filename, #define logPrlError(code) \ logPrlErrorHelper(code, __FILE__, \ - __FUNCTION__, __LINE__) + __FUNCTION__, __LINE__) #define prlsdkCheckRetGoto(ret, label) \ do { \ @@ -129,7 +129,7 @@ logPrlEventErrorHelper(PRL_HANDLE event, const char *filename, #define logPrlEventError(event)\ logPrlEventErrorHelper(event, __FILE__,\ - __FUNCTION__, __LINE__) + __FUNCTION__, __LINE__) static PRL_RESULT getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, @@ -181,7 +181,7 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, #define getJobResult(job, result) \ getJobResultHelper(job, JOB_INFINIT_WAIT_TIMEOUT, \ -result, __FILE__, __FUNCTION__, __LINE__) + result, __FILE__, __FUNCTION__, __LINE__) static PRL_RESULT waitJobHelper(PRL_HANDLE job, unsigned int timeout, @@ -199,7 +199,7 @@ waitJobHelper(PRL_HANDLE job, unsigned int timeout, #define waitJob(job)\ waitJobHelper(job, JOB_INFINIT_WAIT_TIMEOUT, __FILE__, \ - __FUNCTION__, __LINE__) + __FUNCTION__, __LINE__) int @@ -1352,8 +1352,8 @@ prlsdkLoadDomain(vzConnPtr privconn, goto error; if (def-ngraphics 0) { -int bus = IS_CT(def) ? VIR_DOMAIN_INPUT_BUS_PARALLELS: -VIR_DOMAIN_INPUT_BUS_PS2; +int bus = IS_CT(def) ? VIR_DOMAIN_INPUT_BUS_PARALLELS : + VIR_DOMAIN_INPUT_BUS_PS2; if (virDomainDefMaybeAddInput(def, VIR_DOMAIN_INPUT_TYPE_MOUSE, @@ -1377,7 +1377,7 @@ prlsdkLoadDomain(vzConnPtr privconn, if (!(dom = virDomainObjListAdd(privconn-domains, def, privconn-xmlopt, 0, NULL))) -goto error; +goto error; } /* dom is locked here */ @@ -1609,7 +1609,7 @@ prlsdkHandleVmConfigEvent(vzConnPtr privconn, goto cleanup; prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED, -VIR_DOMAIN_EVENT_DEFINED_UPDATED); +VIR_DOMAIN_EVENT_DEFINED_UPDATED); cleanup: virObjectUnlock(dom); @@ -1618,7 +1618,7 @@ prlsdkHandleVmConfigEvent(vzConnPtr privconn, static void prlsdkHandleVmAddedEvent(vzConnPtr privconn, - unsigned char *uuid) + unsigned char *uuid) { virDomainObjPtr dom = NULL; @@ -1627,7 +1627,7 @@ prlsdkHandleVmAddedEvent(vzConnPtr privconn, return; prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED, -VIR_DOMAIN_EVENT_DEFINED_ADDED); +VIR_DOMAIN_EVENT_DEFINED_ADDED); virObjectUnlock(dom); return; @@ -1646,7 +1646,7 @@ prlsdkHandleVmRemovedEvent(vzConnPtr privconn, return; prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_UNDEFINED, -VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); +VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); virDomainObjListRemove(privconn-domains, dom); return; @@ -1656,8 +1656,8 @@ prlsdkHandleVmRemovedEvent(vzConnPtr privconn, static PRL_RESULT prlsdkHandlePerfEvent(vzConnPtr privconn, - PRL_HANDLE event, - unsigned char *uuid) + PRL_HANDLE event, + unsigned char *uuid) { virDomainObjPtr dom = NULL; vzDomObjPtr privdom = NULL; @@ -1668,7 +1668,7 @@ prlsdkHandlePerfEvent(vzConnPtr privconn, goto cleanup; privdom = dom-privateData; -// delayed event after unsubscribe +/* delayed event after unsubscribe */ if (privdom-cache.count == -1) goto cleanup; @@ -1679,12 +1679,12 @@ prlsdkHandlePerfEvent(vzConnPtr privconn, job = PrlVm_UnsubscribeFromPerfStats(privdom-sdkdom); if (PRL_FAILED(waitJob(job))) goto cleanup; -// change state to unsubscribed +/* change state to unsubscribed */ privdom-cache.count = -1; } else { ++privdom-cache.count; privdom-cache.stats = event; -// thus we get own of event handle +/*
[libvirt] [PATCH 0/5] vz: Reformat
There shouldn't be any functional change. Only some formatting nits fixed. Michal Privoznik (5): vz_driver: Reformat vz_network: Reformat vz_sdk: Reformat vz_storage: Reformat vz_utils: Reformat src/vz/vz_driver.c | 86 ++--- src/vz/vz_network.c | 26 +++ src/vz/vz_sdk.c | 209 ++-- src/vz/vz_storage.c | 58 +++ src/vz/vz_utils.h | 20 ++--- 5 files changed, 199 insertions(+), 200 deletions(-) -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] vz_utils: Reformat
Honour our formatting style. Adjust indentation so it matches the rest of our code. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/vz/vz_utils.h | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 9b46bf9..db09647 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -35,7 +35,7 @@ # define vzParseError() \ virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \ - __FUNCTION__, __LINE__, _(Can't parse prlctl output)) + __FUNCTION__, __LINE__, _(Can't parse prlctl output)) # define IS_CT(def) (def-os.type == VIR_DOMAIN_OSTYPE_EXE) @@ -77,8 +77,8 @@ typedef struct _vzConn *vzConnPtr; struct _vzCountersCache { PRL_HANDLE stats; virCond cond; -// -1 - unsubscribed -// -1 - subscribed +/* = -1 - unsubscribed +-1 - subscribed */ int count; }; @@ -115,14 +115,14 @@ char * vzAddFileExt(const char *path, const char *ext); void vzDriverLock(vzConnPtr driver); void vzDriverUnlock(vzConnPtr driver); virStorageVolPtr vzStorageVolLookupByPathLocked(virConnectPtr conn, - const char *path); +const char *path); int vzStorageVolDefRemove(virStoragePoolObjPtr privpool, - virStorageVolDefPtr privvol); + virStorageVolDefPtr privvol); -# define PARALLELS_BLOCK_STATS_FOREACH(OP) \ - OP(rd_req, VIR_DOMAIN_BLOCK_STATS_READ_REQ, read_requests)\ - OP(rd_bytes, VIR_DOMAIN_BLOCK_STATS_READ_BYTES, read_total) \ - OP(wr_req, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ, write_requests) \ - OP(wr_bytes, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, write_total) +# define PARALLELS_BLOCK_STATS_FOREACH(OP) \ +OP(rd_req, VIR_DOMAIN_BLOCK_STATS_READ_REQ, read_requests)\ +OP(rd_bytes, VIR_DOMAIN_BLOCK_STATS_READ_BYTES, read_total) \ +OP(wr_req, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ, write_requests) \ +OP(wr_bytes, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, write_total) #endif -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] vz_driver: Reformat
Honour our formatting style. Adjust indentation so it matches the rest of our code. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/vz/vz_driver.c | 86 +++--- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index d9ddd4f..77cdafd 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -190,8 +190,8 @@ vzConnectGetCapabilities(virConnectPtr conn) static int vzDomainDefPostParse(virDomainDefPtr def, -virCapsPtr caps ATTRIBUTE_UNUSED, -void *opaque ATTRIBUTE_UNUSED) + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { /* memory hotplug tunables are not supported by this driver */ if (virDomainDefCheckUnsupportedMemoryHotplug(def) 0) @@ -202,9 +202,9 @@ vzDomainDefPostParse(virDomainDefPtr def, static int vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, - const virDomainDef *def, - virCapsPtr caps ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) + const virDomainDef *def, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { int ret = -1; @@ -256,7 +256,7 @@ vzOpenDefault(virConnectPtr conn) goto error; if (!(privconn-xmlopt = virDomainXMLOptionNew(vzDomainDefParserConfig, - NULL, NULL))) + NULL, NULL))) goto error; if (!(privconn-domains = virDomainObjListNew())) @@ -289,8 +289,8 @@ vzOpenDefault(virConnectPtr conn) static virDrvOpenStatus vzConnectOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, - unsigned int flags) + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) { int ret; @@ -474,8 +474,8 @@ vzConnectNumOfDefinedDomains(virConnectPtr conn) static int vzConnectListAllDomains(virConnectPtr conn, - virDomainPtr **domains, - unsigned int flags) +virDomainPtr **domains, +unsigned int flags) { vzConnPtr privconn = conn-privateData; int ret = -1; @@ -629,7 +629,7 @@ vzDomainIsPersistent(virDomainPtr domain) static int vzDomainGetState(virDomainPtr domain, - int *state, int *reason, unsigned int flags) + int *state, int *reason, unsigned int flags) { virDomainObjPtr privdom; int ret = -1; @@ -779,7 +779,7 @@ vzDomainDefineXML(virConnectPtr conn, const char *xml) static int vzNodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, - virNodeInfoPtr nodeinfo) + virNodeInfoPtr nodeinfo) { return nodeGetInfo(nodeinfo); } @@ -804,9 +804,9 @@ static int vzConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) static char * vzConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, -const char **xmlCPUs, -unsigned int ncpus, -unsigned int flags) + const char **xmlCPUs, + unsigned int ncpus, + unsigned int flags) { virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); @@ -816,10 +816,10 @@ vzConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, static int vzDomainGetVcpus(virDomainPtr domain, -virVcpuInfoPtr info, -int maxinfo, -unsigned char *cpumaps, -int maplen) + virVcpuInfoPtr info, + int maxinfo, + unsigned char *cpumaps, + int maplen) { virDomainObjPtr privdom = NULL; size_t i; @@ -878,20 +878,20 @@ vzDomainGetVcpus(virDomainPtr domain, static int vzNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED, - unsigned char **cpumap, - unsigned int *online, - unsigned int flags) +unsigned char **cpumap, +unsigned int *online, +unsigned int flags) { return nodeGetCPUMap(cpumap, online, flags); } static int vzConnectDomainEventRegisterAny(virConnectPtr conn, - virDomainPtr domain, - int eventID, - virConnectDomainEventGenericCallback callback, - void *opaque, - virFreeCallback freecb) +virDomainPtr domain, +int
[libvirt] [PATCH 4/5] vz_storage: Reformat
Honour our formatting style. Adjust indentation so it matches the rest of our code. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/vz/vz_storage.c | 58 ++--- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/vz/vz_storage.c b/src/vz/vz_storage.c index 7183db4..1e82195 100644 --- a/src/vz/vz_storage.c +++ b/src/vz/vz_storage.c @@ -48,7 +48,7 @@ static virStorageVolDefPtr vzStorageVolDefineXML(virStoragePoolObjPtr pool, const char *xmldesc, - const char *xmlfile, bool is_new); + const char *xmlfile, bool is_new); static virStorageVolPtr vzStorageVolLookupByPath(virConnectPtr conn, const char *path); @@ -245,8 +245,8 @@ vzPoolAddByDomain(virConnectPtr conn, virDomainObjPtr dom) } static int vzDiskDescParseNode(xmlDocPtr xml, - xmlNodePtr root, - virStorageVolDefPtr def) + xmlNodePtr root, + virStorageVolDefPtr def) { xmlXPathContextPtr ctxt = NULL; int ret = -1; @@ -296,10 +296,10 @@ static int vzDiskDescParse(const char *path, virStorageVolDefPtr def) } static int vzAddDiskVolume(virStoragePoolObjPtr pool, - virDomainObjPtr dom, - const char *diskName, - const char *diskPath, - const char *diskDescPath) + virDomainObjPtr dom, + const char *diskName, + const char *diskPath, + const char *diskDescPath) { virStorageVolDefPtr def = NULL; @@ -332,7 +332,7 @@ static int vzAddDiskVolume(virStoragePoolObjPtr pool, } static int vzFindVmVolumes(virStoragePoolObjPtr pool, - virDomainObjPtr dom) + virDomainObjPtr dom) { vzDomObjPtr pdom = dom-privateData; DIR *dir; @@ -376,7 +376,7 @@ static int vzFindVmVolumes(virStoragePoolObjPtr pool, /* here we know, that ent-d_name is a disk image directory */ if (vzAddDiskVolume(pool, dom, ent-d_name, - diskPath, diskDescPath)) +diskPath, diskDescPath)) goto cleanup; } if (direrr 0) @@ -393,7 +393,7 @@ static int vzFindVmVolumes(virStoragePoolObjPtr pool, static int vzPoolsAdd(virDomainObjPtr dom, - void *opaque) + void *opaque) { virConnectPtr conn = opaque; virStoragePoolObjPtr pool; @@ -464,7 +464,7 @@ static int vzLoadPools(virConnectPtr conn) virDrvOpenStatus vzStorageOpen(virConnectPtr conn, - unsigned int flags) + unsigned int flags) { vzConnPtr privconn = conn-privateData; virStorageDriverStatePtr storageState; @@ -564,7 +564,7 @@ vzConnectNumOfDefinedStoragePools(virConnectPtr conn) static int vzConnectListDefinedStoragePools(virConnectPtr conn, -char **const names, int nnames) + char **const names, int nnames) { vzConnPtr privconn = conn-privateData; int n = 0; @@ -704,7 +704,7 @@ vzStoragePoolGetAlloc(virStoragePoolDefPtr def) static virStoragePoolPtr vzStoragePoolDefineXML(virConnectPtr conn, - const char *xml, unsigned int flags) + const char *xml, unsigned int flags) { vzConnPtr privconn = conn-privateData; virStoragePoolDefPtr def; @@ -1040,7 +1040,7 @@ vzStoragePoolNumOfVolumes(virStoragePoolPtr pool) static int vzStoragePoolListVolumes(virStoragePoolPtr pool, -char **const names, int maxnames) + char **const names, int maxnames) { vzConnPtr privconn = pool-conn-privateData; virStoragePoolObjPtr privpool; @@ -1061,7 +1061,7 @@ vzStoragePoolListVolumes(virStoragePoolPtr pool, if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, - _(storage pool '%s' is not active), pool-name); + _(storage pool '%s' is not active), pool-name); goto error; } @@ -1084,7 +1084,7 @@ vzStoragePoolListVolumes(virStoragePoolPtr pool, static virStorageVolPtr vzStorageVolLookupByName(virStoragePoolPtr pool, - const char *name) + const char *name) { vzConnPtr privconn = pool-conn-privateData; virStoragePoolObjPtr privpool; @@ -1207,8 +1207,8 @@ vzStorageVolLookupByPath(virConnectPtr conn, const char *path) static virStorageVolDefPtr vzStorageVolDefineXML(virStoragePoolObjPtr pool, - const char *xmldesc, - const char *xmlfile, bool is_new) +
Re: [libvirt] [PATCH] qemu: fix wrong remove guest cfg if migrate fail
On Thu, Jun 25, 2015 at 09:38:57 +0800, Luyao Huang wrote: If we get fail in qemuMigrationPrepareAny, we forget check if the vm is persistent then always call qemuDomainRemoveInactive to clean the inactive settings. Add a check to avoid this. This issue was introduce in commit 540c339. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 47d49cd..a57a177 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3432,7 +3432,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_FREE(priv-origname); virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort); priv-nbdPort = 0; -qemuDomainRemoveInactive(driver, vm); +if (!vm-persistent) +qemuDomainRemoveInactive(driver, vm); } virDomainObjEndAPI(vm); if (event) ACK. I rewrote the commit message and pushed this patch, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote: I think the only votes were for option 1 and 4 (interesting how the only ones that were chosen were those that I *didn't* pick personally :-). See comments below. In the meantime the other issue Alex pointed out may cause this to take a slightly different direction. On 06/22/2015 02:44 PM, Laine Stump wrote: === Idea 1: multiplex the meaning of the model attribute, so we currently have: controller type='pci' model='dmi-to-pci-bridge'/ which means add an i82801b11-bridge device and when we add a generic version of this type of controller, we would do it with something like: controller type='pci' model='generic-dmi-to-pci-bridge'/ and for another vendor's mythical controller: controller type='pci' model='xyz-dmi-to-pci-bridge'/ Cons: This will make for ugliness in switch statements where a new case will have to be added whenever a different controller with similar behavior/usage is supported. And it's generally not a good idea to have a single attribute be used for two different functions. jtomko advocated this one because it would make it easier for an older libvirt to notice an unsupported class+model of controller during a migration attempt from a newer libvirt. An example would be if a newer libvirt had a guest with controller type='pci' model='xyz'/ (where xyz is some qemu device that implements a dmi-to-pci-bridge) That would fail on an attempt to migrate to older libvirt, but controller type='pci' model='dmi-to-pci-bridge'/ would succeed. On the other hand, if we did this: controller type='pci' model='dmi-to-pci-bridge submodel='xyz'/ that would succeed (and create the wrong device) because submodel would be ignored. Since there is currently no alternate implementation of a dmi-to-pci-bridge available in qemu, and it will likely be awhile until that happens, I think if we start filling out the XML now as: controller type='pci' model='dmi-to-pci-bridge' submodel='i82801b11-bridge'/ by the time we get to the point that we do have an alternate 'xyz' controller, any version of libvirt that doesn't understand submodel will be so far in the past that we wouldn't want to be able to migrate back to it anyway. === Idea 4: Unlike other uses of model in libvirt, for pci controllers, continue to use model to denote the subtype/class/whatever of controller, and create a new attribute to denote the different specific implementations of each one. So for example: [4] controller type='pci' model='dmi-to-pci-bridge'/ would become: [5] controller type='pci' model='dmi-to-pci-bridge' implementation='i82801b11-bridge'/ (or some other name in place of implementation - ideas? I'm horrible at thinkin up names) Pros: wouldn't create compatibility problems when downgrading or migrating cross version. Cons: Is inconsistent with every other use of model attribute in libvirt, and each new addition of a PCI controller further propagates this misuse. mkletzan preferred this one, and danpb was amenable to it in IRC. I think I now agree, but Alex's comments about needing to keep track of what we put in the qemu commandline for port and chassis have me thinking this isn't enough. The problem is that the ioh3420 device needs its port and chassis options set; Alex recommends setting port to: (slot 3)+function Likewise, he recommends setting port for the xio3130-downstream device to the same value as slot. So, for the following: controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' /controller we would end up with the following commandline: -device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0', addr=0x4.0x1 (chassis is the same as index in the xml (again per Alex's recommendation), and port is (4 3) + 1 = 0x21) *However* he also says that we may change our mind on these in the future, so chassis and port may end up being something different. Since those values are visible to the guest, we can't allow them to change on an existing machine, as it breaks the guest ABI. So we need to store them in the XML. They aren't part of the PCI address though, they are options. So I need to figure out the best way to represent that in the XML, and fill it in when it is auto-generated when the controller is defined. A few possible ways to solve both problems at once: [1] controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' target type='ioh3420' chassis='3' port='0x21'/ /controller Precedent: target is used to store the port number for serial/console devices, specify guest-side bus and device name for disks, specify guest-side mount location for filesystems, specify the *host*-side name of tap devices for network interfaces, memory size for memory. So it seems kind of proper. (slight variation - target model='ioh3420' .../ :-/ ) [2]
Re: [libvirt] [PATCH v2 10/15] test: Finalize removal of locking from driver-eventState
On Wed, Jun 24, 2015 at 14:04:06 -0400, John Ferlan wrote: On 06/24/2015 10:11 AM, Peter Krempa wrote: Don't lock the driver when registering event callbacks. --- src/test/test_driver.c | 12 1 file changed, 12 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2ab0402..b08c1e5 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5915,11 +5915,9 @@ testConnectDomainEventRegister(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virDomainEventStateRegister(conn, driver-eventState, callback, opaque, freecb) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5932,11 +5930,9 @@ testConnectDomainEventDeregister(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virDomainEventStateDeregister(conn, driver-eventState, callback) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5953,12 +5949,10 @@ testConnectDomainEventRegisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret; int ret = 0; Well this certainly isn't material for this patch. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] vz: fix SDK event dispatching
From: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com Current version of SDK event dispatcing is incorrect. For most VM events (add, delete etc) issuer type is PIE_DISPATCHER. Actually analyzing issuer type doesn't have any benifints so this patch get rid of it. All dispatching is done only on event type. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/vz/vz_sdk.c | 58 +++--- 1 files changed, 16 insertions(+), 42 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 98f7a57..2ca74c4 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1697,21 +1697,33 @@ prlsdkHandlePerfEvent(vzConnPtr privconn, return PRL_ERR_SUCCESS; } -static void -prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) +static PRL_RESULT +prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) { +vzConnPtr privconn = opaque; PRL_RESULT pret = PRL_ERR_FAILURE; +PRL_HANDLE_TYPE handleType; char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; unsigned char uuid[VIR_UUID_BUFLEN]; PRL_UINT32 bufsize = ARRAY_CARDINALITY(uuidstr); PRL_EVENT_TYPE prlEventType; -pret = PrlEvent_GetType(prlEvent, prlEventType); +pret = PrlHandle_GetType(prlEvent, handleType); prlsdkCheckRetGoto(pret, cleanup); +/* Currently, there is no need to handle anything but events */ +if (handleType != PHT_EVENT) +goto cleanup; + +if (privconn == NULL) +goto cleanup; + pret = PrlEvent_GetIssuerId(prlEvent, uuidstr, bufsize); prlsdkCheckRetGoto(pret, cleanup); +pret = PrlEvent_GetType(prlEvent, prlEventType); +prlsdkCheckRetGoto(pret, cleanup); + if (prlsdkUUIDParse(uuidstr, uuid) 0) goto cleanup; @@ -1736,44 +1748,7 @@ prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) prlEvent = PRL_INVALID_HANDLE; break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); -} - - cleanup: -PrlHandle_Free(prlEvent); -return; -} - -static PRL_RESULT -prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) -{ -vzConnPtr privconn = opaque; -PRL_RESULT pret = PRL_ERR_FAILURE; -PRL_HANDLE_TYPE handleType; -PRL_EVENT_ISSUER_TYPE prlIssuerType = PIE_UNKNOWN; - -pret = PrlHandle_GetType(prlEvent, handleType); -prlsdkCheckRetGoto(pret, cleanup); - -/* Currently, there is no need to handle anything but events */ -if (handleType != PHT_EVENT) -goto cleanup; - -if (privconn == NULL) -goto cleanup; - -PrlEvent_GetIssuerType(prlEvent, prlIssuerType); -prlsdkCheckRetGoto(pret, cleanup); - -switch (prlIssuerType) { -case PIE_VIRTUAL_MACHINE: -prlsdkHandleVmEvent(privconn, prlEvent); -// above function takes own of event -prlEvent = PRL_INVALID_HANDLE; -break; -default: -VIR_DEBUG(Skipping event of issuer type %d, prlIssuerType); +VIR_DEBUG(Skipping event of type %d, prlEventType); } cleanup: @@ -1781,7 +1756,6 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) return PRL_ERR_SUCCESS; } - int prlsdkSubscribeToPCSEvents(vzConnPtr privconn) { PRL_RESULT pret = PRL_ERR_UNINITIALIZED; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix wrong remove guest cfg if migrate fail
On 06/25/2015 04:25 PM, Jiri Denemark wrote: On Thu, Jun 25, 2015 at 09:38:57 +0800, Luyao Huang wrote: If we get fail in qemuMigrationPrepareAny, we forget check if the vm is persistent then always call qemuDomainRemoveInactive to clean the inactive settings. Add a check to avoid this. This issue was introduce in commit 540c339. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 47d49cd..a57a177 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3432,7 +3432,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_FREE(priv-origname); virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort); priv-nbdPort = 0; -qemuDomainRemoveInactive(driver, vm); +if (!vm-persistent) +qemuDomainRemoveInactive(driver, vm); } virDomainObjEndAPI(vm); if (event) ACK. I rewrote the commit message and pushed this patch, thanks. You are welcome, thanks for your quick review. Jirka Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] vz: Reformat
25.06.2015 11:18, Michal Privoznik пишет: There shouldn't be any functional change. Only some formatting nits fixed. Michal Privoznik (5): vz_driver: Reformat vz_network: Reformat vz_sdk: Reformat vz_storage: Reformat vz_utils: Reformat src/vz/vz_driver.c | 86 ++--- src/vz/vz_network.c | 26 +++ src/vz/vz_sdk.c | 209 ++-- src/vz/vz_storage.c | 58 +++ src/vz/vz_utils.h | 20 ++--- 5 files changed, 199 insertions(+), 200 deletions(-) ACK. Michal, thank you for this work. Maxim Nestratov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 15/15] test: Refactor testNodeGetCPUMap
On Thu, Jun 25, 2015 at 10:57:18 +0200, Pavel Hrdina wrote: On Wed, Jun 24, 2015 at 04:11:57PM +0200, Peter Krempa wrote: Drop locking of the driver since it is not accessed and simplify the code flow. --- src/test/test_driver.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ed67dca..25de641 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5611,31 +5611,23 @@ static int testConnectListAllDomains(virConnectPtr conn, } static int -testNodeGetCPUMap(virConnectPtr conn, +testNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned char **cpumap, unsigned int *online, unsigned int flags) { -testDriverPtr privconn = conn-privateData; -int ret = -1; - virCheckFlags(0, -1); -testDriverLock(privconn); if (cpumap) { if (VIR_ALLOC_N(*cpumap, 1) 0) -goto cleanup; +return -1; *cpumap[0] = 0x15; } if (online) *online = 3; -ret = 8; - - cleanup: -testDriverUnlock(privconn); -return ret; +return 8; It would be nice to have a #define for that value instead of a magic number, but if you decide to leave it here, remove the extra space. Well there's probably just this one usage place, so I don't think it's worth. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vz: Adapt to driver rename
On 06/25/2015 11:14 AM, Michal Privoznik wrote: In the e6d180f07fb06 commit the parallels driver was renamed to vz. However, there was a commit merged later, which was sent to the list before the rename. The other commit is 6de12b026b73. Fix all the missing renames. Sorry, something is broken in my desktop, and libvirt doesn't compile with prlsdk, so I missed it. Thanks for the patch! Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Pushed under build breaker rule. src/vz/vz_sdk.c | 23 --- src/vz/vz_sdk.h | 4 ++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 1a3aa87..388ea19 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2885,14 +2885,20 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, return ret; } -static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) +static int +prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) { +int ret = -1; PRL_RESULT pret; PRL_HANDLE vnet = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; -if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) -return; +if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _(unplugging network device of type %s is not supported), + virDomainNetTypeToString(net-type)); +return ret; +} pret = PrlVirtNet_Create(vnet); prlsdkCheckRetGoto(pret, cleanup); @@ -2904,16 +2910,19 @@ static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) if (PRL_FAILED(pret = waitJob(job))) goto cleanup; +ret = 0; + cleanup: PrlHandle_Free(vnet); +return ret; } int prlsdkAttachNet(virDomainObjPtr dom, -parallelsConnPtr privconn, +vzConnPtr privconn, virDomainNetDefPtr net) { int ret = -1; -parallelsDomObjPtr privdom = dom-privateData; +vzDomObjPtr privdom = dom-privateData; PRL_HANDLE job = PRL_INVALID_HANDLE; if (!IS_CT(dom-def)) { @@ -2998,11 +3007,11 @@ static int prlsdkDelNetAdapter(PRL_HANDLE sdkdom, int idx) } int prlsdkDetachNet(virDomainObjPtr dom, -parallelsConnPtr privconn, +vzConnPtr privconn, virDomainNetDefPtr net) { int ret = -1, idx = -1; -parallelsDomObjPtr privdom = dom-privateData; +vzDomObjPtr privdom = dom-privateData; PRL_HANDLE job = PRL_INVALID_HANDLE; if (!IS_CT(dom-def)) { diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index cde8904..80ff69a 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -67,6 +67,6 @@ prlsdkDetachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk); int prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr stats); int -prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net); +prlsdkAttachNet(virDomainObjPtr dom, vzConnPtr privconn, virDomainNetDefPtr net); int -prlsdkDetachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net); +prlsdkDetachNet(virDomainObjPtr dom, vzConnPtr privconn, virDomainNetDefPtr net); -- Dmitry Guryanov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 15/15] test: Refactor testNodeGetCPUMap
On Wed, Jun 24, 2015 at 04:11:57PM +0200, Peter Krempa wrote: Drop locking of the driver since it is not accessed and simplify the code flow. --- src/test/test_driver.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ed67dca..25de641 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5611,31 +5611,23 @@ static int testConnectListAllDomains(virConnectPtr conn, } static int -testNodeGetCPUMap(virConnectPtr conn, +testNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned char **cpumap, unsigned int *online, unsigned int flags) { -testDriverPtr privconn = conn-privateData; -int ret = -1; - virCheckFlags(0, -1); -testDriverLock(privconn); if (cpumap) { if (VIR_ALLOC_N(*cpumap, 1) 0) -goto cleanup; +return -1; *cpumap[0] = 0x15; } if (online) *online = 3; -ret = 8; - - cleanup: -testDriverUnlock(privconn); -return ret; +return 8; It would be nice to have a #define for that value instead of a magic number, but if you decide to leave it here, remove the extra space. Pavel } static char * -- 2.4.1 -- 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
Re: [libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
On Thu, Jun 25, 2015 at 08:44:17AM +0200, Martin Kletzander wrote: To be honest, I kinds dislike all of them. Not that they would be chosen poorly, no, it's simply because the good sensible choice is unavailable due to another poor decision in the past (this may be another point for Michal's talk on KVM Forum). Thinking about it I must say I don't like how target (which is supposed to match a place where the device appears for the guest) is used for the model specification, on the other hand (ab)using 'model' element for the specification of an address in guest (that's what I understand chassis and port are) doesn't feel any better. What if we go with two of those elements? Would that be too much pain? E.g.: controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' model type='ioh3420'/ target chassis='3' port='0x21'/ /controller I understand this might look like an overkill, but I think it's better safe then sorry, I guess I just see us not so far in the future regretting any decision made now. I'd be fine with this proposal too. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] remote: move connection close callback to driver level
On Wed, Jun 24, 2015 at 05:33:51PM +0300, Nikolay Shirokovskiy wrote: 1. Introduce connect(Un)RegisterCloseCallback driver functions. 2. virConnect(Un)RegisterCloseCallback now works through driver. 3. virConnectCloseCallback is factored from virConnect but mostly stay the same. Notice however that virConnect object is not referenced in virConnectCloseCallback anymore. It is safe. Explanation. Previous version of callback object keeps reference to connection. This leads to undocumented rule that all clients must exlicitly unregister close callback before closing connection or connection will never be disposed. As callback unregistering and close event delivering are serialized thru callback object lock and unregistering zeros connection object we will never get dangling pointer on delivering. 4. callback object doesn't check callback on unregistering. The reason is that it will helps us write registering/unregistering with atomic behaviour for remote driver as it can be seen in next patch. Moreover it is not really meaningful to check callback on unregistering. 5. virNetClientSetCloseCallback call is removed from doRemoteClose as it is excessive for the same reasons as in point 3. Unregistering MUST be called and this prevents from firing event on close initiated by client. I'm not sure where callback object should be so it stays in datatype.c Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/datatypes.c| 112 +--- src/datatypes.h| 21 ++-- src/driver-hypervisor.h| 12 + src/libvirt-host.c | 79 ++- src/remote/remote_driver.c | 67 -- 5 files changed, 179 insertions(+), 112 deletions(-) Lots of spurious whitespace changes, some of them make the commit very messy and make syntax-check fails again after this. Martin signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] vz: Reformat
On Thu, Jun 25, 2015 at 10:18:13AM +0200, Michal Privoznik wrote: There shouldn't be any functional change. Only some formatting nits fixed. Michal Privoznik (5): vz_driver: Reformat vz_network: Reformat vz_sdk: Reformat vz_storage: Reformat vz_utils: Reformat src/vz/vz_driver.c | 86 ++--- src/vz/vz_network.c | 26 +++ src/vz/vz_sdk.c | 209 ++-- src/vz/vz_storage.c | 58 +++ src/vz/vz_utils.h | 20 ++--- 5 files changed, 199 insertions(+), 200 deletions(-) Whitespace-mostly, ACK series. -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list