Re: [libvirt] [PATCH v1 19/31] network_conf: Turn virNetworkObjList into virObject
On 03.03.2015 11:47, Peter Krempa wrote: On Thu, Feb 26, 2015 at 15:17:28 +0100, Michal Privoznik wrote: Well, one day this will be self-locking object, but not today. But lets prepare the code for that! Moreover, virNetworkObjListFree() is no longer needed, so turn it into virNetworkObjListDispose(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- cfg.mk| 1 - src/conf/network_conf.c | 53 +-- src/conf/network_conf.h | 11 src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 5 ++-- src/parallels/parallels_driver.c | 2 +- src/parallels/parallels_network.c | 2 +- src/test/test_driver.c| 13 -- 8 files changed, 56 insertions(+), 33 deletions(-) diff --git a/cfg.mk b/cfg.mk index d72b039..07a794a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -250,7 +250,6 @@ useless_free_options = \ # n virNetworkFree (returns int) # n virNetworkFreeName (returns int) # y virNetworkObjFree -# n virNetworkObjListFree FIXME # n virNodeDevCapsDefFree FIXME # y virNodeDeviceDefFree # n virNodeDeviceFree (returns int) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 24a5f7c..4efad43 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -73,17 +73,33 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName, VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST, hook-script); -bool -virNetworkObjTaint(virNetworkObjPtr obj, - virNetworkTaintFlags taint) I don't see a reason to move this function ... I just wanted the class initiatiors to be together. I prefer having it that way. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 21/31] network_conf: Make virNetworkObj actually virObject
On 03.03.2015 12:05, Peter Krempa wrote:
On Thu, Feb 26, 2015 at 15:17:30 +0100, Michal Privoznik wrote:
So far it's just a structure which happens to have 'Obj' in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
cfg.mk| 2 -
src/conf/network_conf.c | 126
--
src/conf/network_conf.h | 8 +--
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56 -
src/parallels/parallels_network.c | 16 ++---
src/test/test_driver.c| 32 +-
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml| 2 -
9 files changed, 125 insertions(+), 125 deletions(-)
...
@@ -602,17 +620,17 @@ void virNetworkRemoveInactive(virNetworkObjListPtr
nets,
{
size_t i;
-virNetworkObjUnlock(net);
+virObjectUnlock(net);
for (i = 0; i nets-count; i++) {
-virNetworkObjLock(nets-objs[i]);
+virObjectLock(nets-objs[i]);
if (nets-objs[i] == net) {
-virNetworkObjUnlock(nets-objs[i]);
-virNetworkObjFree(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
+virObjectUnref(nets-objs[i]);
VIR_DELETE_ELEMENT(nets-objs, i, nets-count);
break;
}
-virNetworkObjUnlock(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
}
}
The function above is very strange. Your changes should not have any
functional impact, but I think we should fix it right away.
1) why do we unlock the object that is going to be deleted ?
To get the order of locking right.
2) why do we bother locking the objects in between? We are comparing
just the pointer to the object so there's no need to wait for the lock.
Yep, I've saved that for a separate patch, which is not posted yet though.
If we keep the original object locked there's no need to do any of that.
3) the network object is freed _before_ it's removed from the list.
Although the list is always locked before access, I don't think that's a
good practice.
Not anymore. Any API that enters the remove function already has at
least one reference (obtained via virNetworkObjFind*). So this merely
decrease the refcount on the object since the list does no longer hold a
reference to the object.
I'd like to see the function fixed either separately or as a follow up.
I'd also like to see that patch before.
ACK to the rest though, this can be fixed in a individual patch.
Peter
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 19/31] network_conf: Turn virNetworkObjList into virObject
On Tue, Mar 03, 2015 at 13:40:48 +0100, Michal Privoznik wrote: On 03.03.2015 11:47, Peter Krempa wrote: On Thu, Feb 26, 2015 at 15:17:28 +0100, Michal Privoznik wrote: Well, one day this will be self-locking object, but not today. But lets prepare the code for that! Moreover, virNetworkObjListFree() is no longer needed, so turn it into virNetworkObjListDispose(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- cfg.mk| 1 - src/conf/network_conf.c | 53 +-- src/conf/network_conf.h | 11 src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 5 ++-- src/parallels/parallels_driver.c | 2 +- src/parallels/parallels_network.c | 2 +- src/test/test_driver.c| 13 -- 8 files changed, 56 insertions(+), 33 deletions(-) diff --git a/cfg.mk b/cfg.mk index d72b039..07a794a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -250,7 +250,6 @@ useless_free_options = \ # n virNetworkFree (returns int) # n virNetworkFreeName (returns int) # y virNetworkObjFree -# n virNetworkObjListFree FIXME # n virNodeDevCapsDefFree FIXME # y virNodeDeviceDefFree # n virNodeDeviceFree (returns int) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 24a5f7c..4efad43 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -73,17 +73,33 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName, VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST, hook-script); -bool -virNetworkObjTaint(virNetworkObjPtr obj, - virNetworkTaintFlags taint) I don't see a reason to move this function ... I just wanted the class initiatiors to be together. I prefer having it that way. You moved it under the two LookupBy* and there are no class initiator prior to this patch in src/conf/network_conf.c so the statement above doesn't make sense. Anyways, it's not worth bikeshedding about stuff like this. Michal ACK with or without touching virNetworkObjTaint signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 28/31] virNetworkObjFindBy*: Return an reference to found object
On Thu, Feb 26, 2015 at 15:17:37 +0100, Michal Privoznik wrote:
This patch turns both virNetworkObjFindByUUID() and
virNetworkObjFindByName() to return an referenced object so that
even if caller unlocks it, it's for sure that object won't
disappear meanwhile. Especially if the object (in general) is
locked and unlocked during the caller run.
Moreover, this commit is nicely small, since the object unrefing
can be done in virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 11 +++
src/parallels/parallels_network.c | 9 +
src/test/test_driver.c| 4 ++--
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 5b0f36f..c620201 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -137,6 +137,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
return;
virObjectUnlock(*net);
+virObjectUnref(*net);
*net = NULL;
}
@@ -164,6 +165,7 @@ virNetworkObjPtr
virNetworkObjFindByUUID(virNetworkObjListPtr nets,
virObjectLock(nets-objs[i]);
if (!memcmp(nets-objs[i]-def-uuid, uuid, VIR_UUID_BUFLEN)) {
ret = nets-objs[i];
+virObjectRef(ret);
break;
}
virObjectUnlock(nets-objs[i]);
@@ -184,6 +186,7 @@ virNetworkObjPtr
virNetworkObjFindByName(virNetworkObjListPtr nets,
virObjectLock(nets-objs[i]);
if (STREQ(nets-objs[i]-def-name, name)) {
ret = nets-objs[i];
+virObjectRef(ret);
break;
}
virObjectUnlock(nets-objs[i]);
The two hunks above probably won't be a good idea in the light of the
review for 29/31.
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 v1 27/31] virNetworkObjListPtr: Make APIs self-locking
On Thu, Feb 26, 2015 at 15:17:36 +0100, Michal Privoznik wrote:
Every API that touches internal structure of the object must lock
the object first. Not every API that has the object as an
argument needs to do that though. Some APIs just pass the object
to lower layers, which, however, must lock the object then. Look
at the code, you'll get my meaning soon.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 39 +--
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 95d8b4d..5b0f36f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -156,31 +156,41 @@ virNetworkObjListPtr virNetworkObjListNew(void)
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid)
{
+virNetworkObjPtr ret = NULL;
size_t i;
+virObjectLock(nets);
for (i = 0; i nets-count; i++) {
virObjectLock(nets-objs[i]);
-if (!memcmp(nets-objs[i]-def-uuid, uuid, VIR_UUID_BUFLEN))
-return nets-objs[i];
+if (!memcmp(nets-objs[i]-def-uuid, uuid, VIR_UUID_BUFLEN)) {
+ret = nets-objs[i];
+break;
+}
virObjectUnlock(nets-objs[i]);
}
+virObjectUnlock(nets);
-return NULL;
+return ret;
}
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name)
{
+virNetworkObjPtr ret = NULL;
size_t i;
+virObjectLock(nets);
for (i = 0; i nets-count; i++) {
virObjectLock(nets-objs[i]);
-if (STREQ(nets-objs[i]-def-name, name))
-return nets-objs[i];
+if (STREQ(nets-objs[i]-def-name, name)) {
+ret = nets-objs[i];
+break;
+}
virObjectUnlock(nets-objs[i]);
}
+virObjectUnlock(nets);
-return NULL;
+return ret;
}
bool
Later review showed that making the two functions above self locking
won't be ideal.
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 v1 29/31] bridge_driver: Drop networkDriverLock() from almost everywhere
On Thu, Feb 26, 2015 at 15:17:38 +0100, Michal Privoznik wrote:
Now that we have fine grained locks, there's no need to lock the
whole driver. We can rely on self-locking APIs.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/network/bridge_driver.c | 45
+++--
1 file changed, 3 insertions(+), 42 deletions(-)
This patch allows the following race:
Threads 1 and 2 want to define a network with same name. Both threads
call into:
virNetworkObjPtr
virNetworkAssignDef(virNetworkObjListPtr nets,
virNetworkDefPtr def,
bool live)
{
virNetworkObjPtr network;
// both threads are here now
if ((network = virNetworkObjFindByName(nets, def-name))) {
// they both serialize on the virNetworkObjFindByName, but neither of
// those finds the object before the other one manages to add it
virNetworkObjAssignDef(network, def, live);
return network;
}
if (!(network = virNetworkObjNew()))
// now both threads allocate the object ...
return NULL;
virObjectLock(network);
virObjectLock(nets);
// and serialize here again, but both threads think now that they
// shall add the network to the list ...
if (VIR_APPEND_ELEMENT_COPY(nets-objs, nets-count, network) 0)
.. and do so.
goto error;
virNetworkAssignDef() will need more love. I think that
virNetworkObjFindByName and friends should not lock or ref the network
object and the callers such as virNetworkAssignDef() or
networkObjFromNetwork() should do it.
In that way, you'll be able to lock the list before and TEST_AND_SET the
object in an atomic fashion.
Rest of this patch looks okay.
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] docs: add a mention for start a vm with rawio = 'yes'
On 03/02/2015 05:43 AM, Daniel P. Berrange wrote: On Mon, Mar 02, 2015 at 06:04:44PM +0800, Luyao Huang wrote: When we start a vm which have rawio = 'yes' settings without any file caps settings for qemu, qemu process still cannot use this caps (CAP_SYS_RAWIO) and the /proc/pidofqemu/status like this: CapInh: 0002 CapPrm: CapEff: CapBnd: 001f this is because we do not set file caps for qemu (see man 7 capabilities), although laine have mentioned this in commit e11451, i think it will be good if we add this in docs. This is only true if you are starting the guest under the qemu:///session URI. In such a case I think it is expected that the QEMU lacks rawio capabilities, because the whole point of qemu:///session is that the VM has no elevated privileges. In the case of qemu:///system libvirt should ensure that it does the right thing with passing on raw io capability flag. If it does not, then we must fix that in the code, not the docs. libvirt does do the right thing as much as can be done. The commit Luyao references above has a summary of what I learned at the time I did this code (I don't remember who I verified that information with after my experiments failed to get the cap set, but it was somebody who understands capability bits better than me :-) Basically, the problem is that, in order for CapPrm and CapEff to have a bit set, the executable file (e.g. qemu-x86_64) itself must have that capability bit set. So libvirt's choices are: 1) require qemu to set the CAP_SYS_RAWIO bit on all their executables (NB: this is the only way hotplug of devices requiring CAP_SYS_RAWIO can work, since you can't *add* a capability to a process once it has been removed) (NB2: This is really beyond reasonable, since the vast majority of domains don't need CAP_SYS_RAWIO and it's not reasonable to give all of them a larger exposure to potential security problems just in case somebody someday might want to hotplug a scsi device with rawio) 2) keep track of how many active domains require CAP_SYS_RAWIO for each qemu binary, and set/clear that bit for the binary as required (still unacceptable - while we change the permissions/ownership of disk images and sockets, I don't think we should be changing the capability bits of system binaries in /usr/bin) 3) require the admin to set the CAP_SYS_RAWIO cap for the qemu binary if they are going to use it. In all cases, libvirt still needs to keep the CAP_SYS_RAWIO cap for the qemu processes that will actually use it, but this is something required *in addition to*, not instead of, setting the bit for the file. As ugly and inconvenient as it is, setting the cap bit on the qemu executable really is necessary to use CAP_SYS_RAWIO (unless my information + experiments were wrong), so I think it's reasonable to add this note (or something equivalent) to the documentation (I should have done so at the time, but as usual was thinking more about the code than about documenting what it did) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix not remove the pidfile when close a vm after restart libvirtd
On 02.03.2015 10:37, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1197600
when create a happy vm and then restart libvirtd, we will loss
priv-pidfile, because we don't check if it is there is a pidfile.
However we only use this pidfile when we start the vm, and won't use
it after it start, so this is not a big deal.
But it is strange when vm is offline but pidfile still exist, so
remove vmname.pid in state dir (maybe /run/libvirt/qemu/)when
priv-pidfile is NULL.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_process.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 515402e..46b93b3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -92,6 +92,7 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
{
char ebuf[1024];
char *file = NULL;
+char *pidfile = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = -1;
@@ -99,16 +100,19 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
if (virAsprintf(file, %s/%s.xml, cfg-stateDir, vm-def-name) 0)
goto cleanup;
+if (virAsprintf(pidfile, %s/%s.pid, cfg-stateDir, vm-def-name) 0)
+goto cleanup;
If this fails, @file is leaked. And there's a helper function to
generate pid file path: virPidFileBuildPath(). I know it does exactly
the same, but lets use that, so whenever we decide on changing the path,
we need to change it at one place only, instead of digging through
source code just to check if somebody has not used virAsprintf() directly.
+
if (unlink(file) 0 errno != ENOENT errno != ENOTDIR)
VIR_WARN(Failed to remove domain XML for %s: %s,
vm-def-name, virStrerror(errno, ebuf, sizeof(ebuf)));
VIR_FREE(file);
-if (priv-pidfile
-unlink(priv-pidfile) 0
+if (unlink(priv-pidfile ? priv-pidfile : pidfile) 0
errno != ENOENT)
VIR_WARN(Failed to remove PID file for %s: %s,
vm-def-name, virStrerror(errno, ebuf, sizeof(ebuf)));
+VIR_FREE(pidfile);
ret = 0;
cleanup:
While this works, I think we need a different approach:
https://www.redhat.com/archives/libvir-list/2015-March/msg00047.html
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 19/31] network_conf: Turn virNetworkObjList into virObject
On Thu, Feb 26, 2015 at 15:17:28 +0100, Michal Privoznik wrote:
Well, one day this will be self-locking object, but not today.
But lets prepare the code for that! Moreover,
virNetworkObjListFree() is no longer needed, so turn it into
virNetworkObjListDispose().
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
cfg.mk| 1 -
src/conf/network_conf.c | 53
+--
src/conf/network_conf.h | 11
src/libvirt_private.syms | 2 +-
src/network/bridge_driver.c | 5 ++--
src/parallels/parallels_driver.c | 2 +-
src/parallels/parallels_network.c | 2 +-
src/test/test_driver.c| 13 --
8 files changed, 56 insertions(+), 33 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index d72b039..07a794a 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -250,7 +250,6 @@ useless_free_options =\
# n virNetworkFree (returns int)
# n virNetworkFreeName (returns int)
# y virNetworkObjFree
-# n virNetworkObjListFree FIXME
# n virNodeDevCapsDefFree FIXME
# y virNodeDeviceDefFree
# n virNodeDeviceFree (returns int)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 24a5f7c..4efad43 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -73,17 +73,33 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName,
VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST,
hook-script);
-bool
-virNetworkObjTaint(virNetworkObjPtr obj,
- virNetworkTaintFlags taint)
I don't see a reason to move this function ...
+static virClassPtr virNetworkObjListClass;
+static void virNetworkObjListDispose(void *obj);
+
+static int virNetworkObjOnceInit(void)
+{
+if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
+ virNetworkObjList,
+ sizeof(virNetworkObjList),
+ virNetworkObjListDispose)))
+return -1;
+return 0;
+}
+
+
+VIR_ONCE_GLOBAL_INIT(virNetworkObj)
+
+virNetworkObjListPtr virNetworkObjListNew(void)
{
-unsigned int flag = (1 taint);
+virNetworkObjListPtr nets;
+
+if (virNetworkObjInitialize() 0)
+return NULL;
-if (obj-taint flag)
-return false;
+if (!(nets = virObjectNew(virNetworkObjListClass)))
+return NULL;
-obj-taint |= flag;
-return true;
+return nets;
}
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
@@ -116,6 +132,19 @@ virNetworkObjPtr
virNetworkObjFindByName(virNetworkObjListPtr nets,
return NULL;
}
+bool
+virNetworkObjTaint(virNetworkObjPtr obj,
+ virNetworkTaintFlags taint)
Here.
+{
+unsigned int flag = (1 taint);
+
+if (obj-taint flag)
+return false;
+
+obj-taint |= flag;
+return true;
+}
+
static void
virPortGroupDefClear(virPortGroupDefPtr def)
@@ -275,18 +304,16 @@ void virNetworkObjFree(virNetworkObjPtr net)
VIR_FREE(net);
}
-void virNetworkObjListFree(virNetworkObjListPtr nets)
+static void
+virNetworkObjListDispose(void *obj)
{
+virNetworkObjListPtr nets = obj;
size_t i;
-if (!nets)
-return;
-
for (i = 0; i nets-count; i++)
virNetworkObjFree(nets-objs[i]);
VIR_FREE(nets-objs);
-nets-count = 0;
}
/*
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 164fb1a..5725258 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -40,6 +40,7 @@
# include device_conf.h
# include virbitmap.h
# include networkcommon_conf.h
+# include virobject.h
typedef enum {
VIR_NETWORK_FORWARD_NONE = 0,
@@ -277,6 +278,8 @@ struct _virNetworkObj {
typedef struct _virNetworkObjList virNetworkObjList;
typedef virNetworkObjList *virNetworkObjListPtr;
struct _virNetworkObjList {
+virObject parent;
+
size_t count;
virNetworkObjPtr *objs;
};
@@ -298,19 +301,17 @@ virNetworkObjIsActive(const virNetworkObj *net)
return net-active;
}
-bool virNetworkObjTaint(virNetworkObjPtr obj,
-virNetworkTaintFlags taint);
Nor this definition ...
+virNetworkObjListPtr virNetworkObjListNew(void);
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid);
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name);
-
+bool virNetworkObjTaint(virNetworkObjPtr obj,
+virNetworkTaintFlags taint);
Here.
void virNetworkDefFree(virNetworkDefPtr def);
void virNetworkObjFree(virNetworkObjPtr net);
-void virNetworkObjListFree(virNetworkObjListPtr nets);
-
typedef bool
[libvirt] [PATCH] qemuProcessReconnect: Fill in pid file path
https://bugzilla.redhat.com/show_bug.cgi?id=1197600
So, libvirt uses pid file to track pid of started qemus. Whenever
a domain is started, its pid is put into corresponding pid file.
The pid file path is generated based on domain name and stored
into domain object internals. However, it's not stored in the
status XML and therefore lost on daemon restarts. Hence, later,
when domain is being shut down, the daemon does not know which
pid file to unlink, and the correct pid file is left behind. To
avoid this, lets generate the pid file path again in
qemuProcessReconnect().
Reported-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_process.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7ce6115..e77616a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3762,6 +3762,13 @@ qemuProcessReconnect(void *opaque)
cfg = virQEMUDriverGetConfig(driver);
priv = obj-privateData;
+/* XXX If we ever gonna change pid file pattern, come up with
+ * some intelligence here to deal with old paths. */
+if (!(priv-pidfile = virPidFileBuildPath(cfg-stateDir, obj-def-name)))
{
+virReportSystemError(errno, %s, _(Failed to build pidfile path.));
+goto killvm;
+}
+
if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) 0)
goto killvm;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] PowerPC : Miscellaneous fixes for 'ppc64le' architecture.
On Tuesday 03 March 2015 03:29 PM, Michal Privoznik wrote: On 26.02.2015 18:09, Prerna Saxena wrote: From a28ef5a3e7b9cb023948cf97d9f472bb3a1e06d3 Mon Sep 17 00:00:00 2001 From: Prerna Saxena pre...@linux.vnet.ibm.com Date: Thu, 26 Feb 2015 22:31:05 +0530 This series adds few miscellaneous fixes for PowerPC 64-bit Little Endian Architecture. Changelog : == v1 of Patch 1/2 already posted and acked : https://www.redhat.com/archives/libvir-list/2015-February/msg00486.html Patch 2/2 adds a testcase to supplement patch 1. Prerna Saxena (2): PowerPC: Augment XML schema to include 'ppc64le' arch and newer pseries-2.* machine types. Tests : Add test for 'ppc64le' architecture. docs/schemas/basictypes.rng| 1 + docs/schemas/domaincommon.rng | 7 - .../qemuxml2argv-pseries-cpu-le.args | 7 + .../qemuxml2argv-pseries-cpu-le.xml| 17 tests/qemuxml2argvtest.c | 2 ++ tests/testutilsqemu.c | 30 ++ 6 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml I've tweaked the commit messages a bit, ACKed and pushed. Michal Thank you :-) -- 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] qemuProcessReconnect: Fill in pid file path
On Tue, Mar 03, 2015 at 11:56:37 +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1197600
So, libvirt uses pid file to track pid of started qemus. Whenever
a domain is started, its pid is put into corresponding pid file.
The pid file path is generated based on domain name and stored
into domain object internals. However, it's not stored in the
status XML and therefore lost on daemon restarts. Hence, later,
when domain is being shut down, the daemon does not know which
pid file to unlink, and the correct pid file is left behind. To
avoid this, lets generate the pid file path again in
qemuProcessReconnect().
Right, this is what I wanted to suggest after seeing the original patch.
Reported-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_process.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7ce6115..e77616a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3762,6 +3762,13 @@ qemuProcessReconnect(void *opaque)
cfg = virQEMUDriverGetConfig(driver);
priv = obj-privateData;
+/* XXX If we ever gonna change pid file pattern, come up with
+ * some intelligence here to deal with old paths. */
+if (!(priv-pidfile = virPidFileBuildPath(cfg-stateDir,
obj-def-name))) {
+virReportSystemError(errno, %s, _(Failed to build pidfile
path.));
Just remove this line. virPidFileBuildPath already reports the error.
+goto killvm;
+}
+
if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) 0)
goto killvm;
ACK
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 21/31] network_conf: Make virNetworkObj actually virObject
On Thu, Feb 26, 2015 at 15:17:30 +0100, Michal Privoznik wrote:
So far it's just a structure which happens to have 'Obj' in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
cfg.mk| 2 -
src/conf/network_conf.c | 126
--
src/conf/network_conf.h | 8 +--
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56 -
src/parallels/parallels_network.c | 16 ++---
src/test/test_driver.c| 32 +-
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml| 2 -
9 files changed, 125 insertions(+), 125 deletions(-)
...
@@ -602,17 +620,17 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
{
size_t i;
-virNetworkObjUnlock(net);
+virObjectUnlock(net);
for (i = 0; i nets-count; i++) {
-virNetworkObjLock(nets-objs[i]);
+virObjectLock(nets-objs[i]);
if (nets-objs[i] == net) {
-virNetworkObjUnlock(nets-objs[i]);
-virNetworkObjFree(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
+virObjectUnref(nets-objs[i]);
VIR_DELETE_ELEMENT(nets-objs, i, nets-count);
break;
}
-virNetworkObjUnlock(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
}
}
The function above is very strange. Your changes should not have any
functional impact, but I think we should fix it right away.
1) why do we unlock the object that is going to be deleted ?
2) why do we bother locking the objects in between? We are comparing
just the pointer to the object so there's no need to wait for the lock.
If we keep the original object locked there's no need to do any of that.
3) the network object is freed _before_ it's removed from the list.
Although the list is always locked before access, I don't think that's a
good practice.
I'd like to see the function fixed either separately or as a follow up.
I'd also like to see that patch before.
ACK to the rest though, this can be fixed in a individual patch.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 6/7] Add gvir_designer_domain_interface_{bridge, user} methods
These cover 2 additional types of libvirt interfaces: usermode/SLIRP
networking and bridge.
---
libvirt-designer/libvirt-designer-domain.c | 70 +-
libvirt-designer/libvirt-designer-domain.h | 5 +++
libvirt-designer/libvirt-designer.sym | 2 +
3 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/libvirt-designer/libvirt-designer-domain.c
b/libvirt-designer/libvirt-designer-domain.c
index 0321180..a060a81 100644
--- a/libvirt-designer/libvirt-designer-domain.c
+++ b/libvirt-designer/libvirt-designer-domain.c
@@ -54,7 +54,9 @@ G_DEFINE_TYPE(GVirDesignerDomain, gvir_designer_domain,
G_TYPE_OBJECT);
#define GVIR_DESIGNER_DOMAIN_ERROR gvir_designer_domain_error_quark()
typedef enum {
+GVIR_DESIGNER_DOMAIN_NIC_TYPE_BRIDGE,
GVIR_DESIGNER_DOMAIN_NIC_TYPE_NETWORK,
+GVIR_DESIGNER_DOMAIN_NIC_TYPE_USER,
/* add new type here */
} GVirDesignerDomainNICType;
@@ -1869,7 +1871,7 @@ cleanup:
static GVirConfigDomainInterface *
gvir_designer_domain_add_interface_full(GVirDesignerDomain *design,
GVirDesignerDomainNICType type,
-const char *network,
+const char *source,
GError **error)
{
GVirConfigDomainInterface *ret = NULL;
@@ -1878,10 +1880,18 @@
gvir_designer_domain_add_interface_full(GVirDesignerDomain *design,
model = gvir_designer_domain_get_preferred_nic_model(design, error);
switch (type) {
+case GVIR_DESIGNER_DOMAIN_NIC_TYPE_BRIDGE:
+ret =
GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_bridge_new());
+
gvir_config_domain_interface_bridge_set_source(GVIR_CONFIG_DOMAIN_INTERFACE_BRIDGE(ret),
+ source);
+break;
case GVIR_DESIGNER_DOMAIN_NIC_TYPE_NETWORK:
ret =
GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());
gvir_config_domain_interface_network_set_source(GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(ret),
-network);
+source);
+break;
+case GVIR_DESIGNER_DOMAIN_NIC_TYPE_USER:
+ret =
GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_user_new());
break;
default:
g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0,
@@ -1898,6 +1908,35 @@ cleanup:
return ret;
}
+/**
+ * gvir_designer_domain_add_interface_bridge:
+ * @design: (transfer none): the domain designer instance
+ * @bridge: (transfer none): bridge name
+ * @error: return location for a #GError, or NULL
+ *
+ * Add new network interface card into @design. The interface is
+ * of 'bridge' type and uses @bridge as the bridge device
+ *
+ * Returns: (transfer full): the pointer to the new interface.
+ */
+GVirConfigDomainInterface *
+gvir_designer_domain_add_interface_bridge(GVirDesignerDomain *design,
+ const char *bridge,
+ GError **error)
+{
+g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL);
+g_return_val_if_fail(bridge != NULL, NULL);
+g_return_val_if_fail(!error_is_set(error), NULL);
+
+GVirConfigDomainInterface *ret = NULL;
+
+ret = gvir_designer_domain_add_interface_full(design,
+
GVIR_DESIGNER_DOMAIN_NIC_TYPE_BRIDGE,
+ bridge,
+ error);
+
+return ret;
+}
/**
* gvir_designer_domain_add_interface_network:
@@ -1929,6 +1968,33 @@
gvir_designer_domain_add_interface_network(GVirDesignerDomain *design,
return ret;
}
+/**
+ * gvir_designer_domain_add_interface_user:
+ * @design: (transfer none): the domain designer instance
+ * @error: return location for a #GError, or NULL
+ *
+ * Add new network interface card into @design. The interface is
+ * of 'user' type.
+ *
+ * Returns: (transfer full): the pointer to the new interface.
+ */
+GVirConfigDomainInterface *
+gvir_designer_domain_add_interface_user(GVirDesignerDomain *design,
+GError **error)
+{
+g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL);
+g_return_val_if_fail(!error_is_set(error), NULL);
+
+GVirConfigDomainInterface *ret = NULL;
+
+ret = gvir_designer_domain_add_interface_full(design,
+
GVIR_DESIGNER_DOMAIN_NIC_TYPE_USER,
+ NULL,
+ error);
+
+return ret;
+}
+
static GVirConfigDomainVideoModel
gvir_designer_domain_video_model_str_to_enum(const char *model_str,
GError **error)
diff --git
[libvirt] [libvirt-designer 5/7] Add more preconditions to gvir_designer_domain_add_interface_network()
---
libvirt-designer/libvirt-designer-domain.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libvirt-designer/libvirt-designer-domain.c
b/libvirt-designer/libvirt-designer-domain.c
index 7387ab1..0321180 100644
--- a/libvirt-designer/libvirt-designer-domain.c
+++ b/libvirt-designer/libvirt-designer-domain.c
@@ -1916,6 +1916,8 @@
gvir_designer_domain_add_interface_network(GVirDesignerDomain *design,
GError **error)
{
g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL);
+g_return_val_if_fail(network != NULL, NULL);
+g_return_val_if_fail(!error_is_set(error), NULL);
GVirConfigDomainInterface *ret = NULL;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 0/7]
Hey, After seeing the libvirt-designer GSoC idea, I remembered about a bunch of libvirt-designer patches I had never sent :) Here they are, a few leak fixes/build cleanups, and addition of 2 new methods for completeness. Christophe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 7/7] Fix GVirConfigDomainDiskDriver leak
A GVirConfigDomainDiskDriver is created in gvir_designer_domain_add_disk_full but it's never unref'ed. This commit fixes that, including when an error occurs. --- libvirt-designer/libvirt-designer-domain.c | 4 1 file changed, 4 insertions(+) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index a060a81..c1a0e83 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -1626,6 +1626,8 @@ gvir_designer_domain_add_disk_full(GVirDesignerDomain *design, gvir_config_domain_disk_set_type(disk, type); gvir_config_domain_disk_set_source(disk, path); gvir_config_domain_disk_set_driver(disk, driver); +g_object_unref(driver); +driver = NULL; controller = gvir_designer_domain_get_preferred_disk_controller(design, NULL); if (controller == NULL) @@ -1656,6 +1658,8 @@ gvir_designer_domain_add_disk_full(GVirDesignerDomain *design, error: g_free(target_gen); +if (driver != NULL) +g_object_unref(driver); if (disk) g_object_unref(disk); return NULL; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 1/7] build-sys: Use AM_CPPFLAGS instead of INCLUDES
The latter is deprecated in favour of the former. --- examples/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/Makefile.am b/examples/Makefile.am index 1d8947f..65a5afe 100644 --- a/examples/Makefile.am +++ b/examples/Makefile.am @@ -1,4 +1,4 @@ -INCLUDES = \ +AM_CPPFLAGS = \ -I$(top_builddir)/libvirt-designer \ -I$(top_srcdir) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 2/7] build-sys: Update manywarnings.m4 from gnulib
This fixes gcc warning about -Wmudflap on fedora 21
---
m4/manywarnings.m4 | 213 ++---
1 file changed, 137 insertions(+), 76 deletions(-)
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index fd0e372..3e6dd21 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -1,5 +1,5 @@
-# manywarnings.m4 serial 3
-dnl Copyright (C) 2008-2012 Free Software Foundation, Inc.
+# manywarnings.m4 serial 7
+dnl Copyright (C) 2008-2014 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
@@ -35,14 +35,12 @@ AC_DEFUN([gl_MANYWARN_COMPLEMENT],
# make sure your gcc understands it.
AC_DEFUN([gl_MANYWARN_ALL_GCC],
[
- dnl First, check if -Wno-missing-field-initializers is needed.
- dnl -Wmissing-field-initializers is implied by -W, but that issues
- dnl warnings with GCC version before 4.7, for the common idiom
- dnl of initializing types on the stack to zero, using { 0, }
+ dnl First, check for some issues that only occur when combining multiple
+ dnl gcc warning categories.
AC_REQUIRE([AC_PROG_CC])
if test -n $GCC; then
-dnl First, check -W -Werror -Wno-missing-field-initializers is supported
+dnl Check if -W -Werror -Wno-missing-field-initializers is supported
dnl with the current $CC $CFLAGS $CPPFLAGS.
AC_MSG_CHECKING([whether -Wno-missing-field-initializers is supported])
AC_CACHE_VAL([gl_cv_cc_nomfi_supported], [
@@ -77,108 +75,171 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
])
AC_MSG_RESULT([$gl_cv_cc_nomfi_needed])
fi
+
+dnl Next, check if -Werror -Wuninitialized is useful with the
+dnl user's choice of $CFLAGS; some versions of gcc warn that it
+dnl has no effect if -O is not also used
+AC_MSG_CHECKING([whether -Wuninitialized is supported])
+AC_CACHE_VAL([gl_cv_cc_uninitialized_supported], [
+ gl_save_CFLAGS=$CFLAGS
+ CFLAGS=$CFLAGS -Werror -Wuninitialized
+ AC_COMPILE_IFELSE(
+[AC_LANG_PROGRAM([[]], [[]])],
+[gl_cv_cc_uninitialized_supported=yes],
+[gl_cv_cc_uninitialized_supported=no])
+ CFLAGS=$gl_save_CFLAGS])
+AC_MSG_RESULT([$gl_cv_cc_uninitialized_supported])
+
fi
+ # List all gcc warning categories.
+ # To compare this list to your installed GCC's, run this Bash command:
+ #
+ # comm -3 \
+ # (sed -n 's/^ *\(-[^ ]*\) .*/\1/p' manywarnings.m4 | sort) \
+ # (gcc --help=warnings | sed -n 's/^ \(-[^ ]*\) .*/\1/p' | sort |
+ # grep -v -x -f (
+ # awk '/^[^#]/ {print $1}' ../build-aux/gcc-warning.spec))
+
gl_manywarn_set=
for gl_manywarn_item in \
--Wall \
-W \
--Wformat-y2k \
--Wformat-nonliteral \
--Wformat-security \
--Winit-self \
--Wmissing-include-dirs \
--Wswitch-default \
--Wswitch-enum \
--Wunused \
--Wunknown-pragmas \
--Wstrict-aliasing \
--Wstrict-overflow \
--Wsystem-headers \
--Wfloat-equal \
--Wtraditional \
--Wtraditional-conversion \
--Wdeclaration-after-statement \
--Wundef \
--Wshadow \
--Wunsafe-loop-optimizations \
--Wpointer-arith \
+-Wabi \
+-Waddress \
+-Waggressive-loop-optimizations \
+-Wall \
+-Warray-bounds \
+-Wattributes \
-Wbad-function-cast \
--Wc++-compat \
--Wcast-qual \
--Wcast-align \
--Wwrite-strings \
--Wconversion \
--Wsign-conversion \
--Wlogical-op \
--Waggregate-return \
--Wstrict-prototypes \
--Wold-style-definition \
--Wmissing-prototypes \
--Wmissing-declarations \
--Wmissing-noreturn \
--Wmissing-format-attribute \
--Wpacked \
--Wpadded \
--Wredundant-decls \
--Wnested-externs \
--Wunreachable-code \
--Winline \
--Winvalid-pch \
--Wlong-long \
--Wvla \
--Wvolatile-register-var \
--Wdisabled-optimization \
--Wstack-protector \
--Woverlength-strings \
-Wbuiltin-macro-redefined \
--Wmudflap \
--Wpacked-bitfield-compat \
--Wsync-nand \
-; do
-gl_manywarn_set=$gl_manywarn_set $gl_manywarn_item
- done
- # The following are not documented in the manual but are included in
- # output from gcc --help=warnings.
- for gl_manywarn_item in \
--Wattributes \
+-Wcast-align \
+-Wchar-subscripts \
+-Wclobbered \
+-Wcomment \
+-Wcomments \
-Wcoverage-mismatch \
--Wmultichar \
--Wunused-macros \
-; do
-gl_manywarn_set=$gl_manywarn_set $gl_manywarn_item
- done
- # More warnings from gcc 4.6.2 --help=warnings.
- for gl_manywarn_item in \
--Wabi \
-Wcpp \
+-Wdate-time \
-Wdeprecated \
-Wdeprecated-declarations \
+-Wdisabled-optimization \
-Wdiv-by-zero \
-Wdouble-promotion \
+-Wempty-body \
-Wendif-labels \
+-Wenum-compare \
-Wextra \
[libvirt] [libvirt-designer 4/7] Don't return uninitialized memory from gvir_designer_domain_add_interface_full()
When an unknown NIC type is passed to
gvir_designer_domain_add_interface_full(), 'ret' would be returned
uninitialized to the caller.
---
libvirt-designer/libvirt-designer-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libvirt-designer/libvirt-designer-domain.c
b/libvirt-designer/libvirt-designer-domain.c
index 5475c79..7387ab1 100644
--- a/libvirt-designer/libvirt-designer-domain.c
+++ b/libvirt-designer/libvirt-designer-domain.c
@@ -1872,7 +1872,7 @@
gvir_designer_domain_add_interface_full(GVirDesignerDomain *design,
const char *network,
GError **error)
{
-GVirConfigDomainInterface *ret;
+GVirConfigDomainInterface *ret = NULL;
const gchar *model = NULL;
model = gvir_designer_domain_get_preferred_nic_model(design, error);
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 3/7] virtxml: Don't leak 'iface' in add_iface()
The caller of gvir_designer_domain_add_interface_network() owns a reference on the returned GVirConfigDomainInterface instance, so it needs to be released when no longer needed. --- examples/virt-designer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/virt-designer.c b/examples/virt-designer.c index 7628449..f134b98 100644 --- a/examples/virt-designer.c +++ b/examples/virt-designer.c @@ -350,6 +350,7 @@ add_iface(gpointer data, exit(EXIT_FAILURE); } } +g_object_unref(iface); } static gboolean -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] schema: Fix interface link state schema
On Mon, Mar 02, 2015 at 10:37:06 -0700, Eric Blake wrote: On 03/02/2015 09:40 AM, Peter Krempa wrote: In commit edd1295e1da6bfe8e4e257e5fbfad71ac0bf7c87 I've introduced an XML element that allows to configure state of the network interface link. Somehow the RNG schema hunk ended up in a weird place in the network schema definition. Move it to the right place and add a test case. Note that the link state is set up via the monitor at VM startup so I originally didn't think of adding a test case. Even when the command line is not impacted, testing the xml2xml path is a good thing to add :) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1173468 --- docs/schemas/domaincommon.rng| 12 docs/schemas/network.rng | 11 --- tests/qemuxml2argvdata/qemuxml2argv-interface-driver.xml | 1 + 3 files changed, 13 insertions(+), 11 deletions(-) ACK Pushed; Thanks. 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 v1 21/31] network_conf: Make virNetworkObj actually virObject
On Tue, Mar 03, 2015 at 13:47:32 +0100, Michal Privoznik wrote:
On 03.03.2015 12:05, Peter Krempa wrote:
On Thu, Feb 26, 2015 at 15:17:30 +0100, Michal Privoznik wrote:
So far it's just a structure which happens to have 'Obj' in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
cfg.mk| 2 -
src/conf/network_conf.c | 126
--
src/conf/network_conf.h | 8 +--
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56 -
src/parallels/parallels_network.c | 16 ++---
src/test/test_driver.c| 32 +-
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml| 2 -
9 files changed, 125 insertions(+), 125 deletions(-)
...
@@ -602,17 +620,17 @@ void virNetworkRemoveInactive(virNetworkObjListPtr
nets,
{
size_t i;
-virNetworkObjUnlock(net);
+virObjectUnlock(net);
for (i = 0; i nets-count; i++) {
-virNetworkObjLock(nets-objs[i]);
+virObjectLock(nets-objs[i]);
if (nets-objs[i] == net) {
-virNetworkObjUnlock(nets-objs[i]);
-virNetworkObjFree(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
+virObjectUnref(nets-objs[i]);
VIR_DELETE_ELEMENT(nets-objs, i, nets-count);
break;
}
-virNetworkObjUnlock(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
}
}
The function above is very strange. Your changes should not have any
functional impact, but I think we should fix it right away.
1) why do we unlock the object that is going to be deleted ?
To get the order of locking right.
Well, this would make it only simpler to deal with locking in the loop
below since the code always grabs the network driver lock for every
operation. And since ...
2) why do we bother locking the objects in between? We are comparing
just the pointer to the object so there's no need to wait for the lock.
Yep, I've saved that for a separate patch, which is not posted yet though.
... locking of the object doesn't make sense there shouldn't be any
issue.
If we keep the original object locked there's no need to do any of that.
3) the network object is freed _before_ it's removed from the list.
Although the list is always locked before access, I don't think that's a
good practice.
Not anymore. Any API that enters the remove function already has at
least one reference (obtained via virNetworkObjFind*). So this merely
decrease the refcount on the object since the list does no longer hold a
reference to the object.
That is not true at this point. At this point in the series there's only
one reference ever in the list so that the object gets deleted in this
function. You add reference counting in patch 28/31.
I'd like to see the function fixed either separately or as a follow up.
I'd also like to see that patch before.
ACK to the rest though, this can be fixed in a individual patch.
Peter
Michal
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 23/31] network_conf: Introduce virNetworkObjEndAPI
On Thu, Feb 26, 2015 at 15:17:32 +0100, Michal Privoznik wrote:
This is practically copy of qemuDomObjEndAPI. The reason why is
it so widely available is to avoid code duplication, since the
function is going to be called from our bridge driver, test
driver and parallels driver too.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 13 +++--
src/conf/network_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 7e8b867..95d8b4d 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -130,6 +130,16 @@ virNetworkObjNew(void)
return NULL;
}
+void
+virNetworkObjEndAPI(virNetworkObjPtr *net)
+{
+if (!*net)
+return;
+
+virObjectUnlock(*net);
+*net = NULL;
+}
+
virNetworkObjListPtr virNetworkObjListNew(void)
{
virNetworkObjListPtr nets;
@@ -4240,8 +4250,7 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets,
}
cleanup:
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(net);
return ret;
}
You should also convert the onle place in virNetworkLoadAllConfigs() to
this new helper since it will also return a referenced object. I'll
point out the problem later.
ACK with virNetworkLoadAllConfigs tweaked too.
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 v1 24/31] bridge_driver: Use virNetworkObjEndAPI
On Thu, Feb 26, 2015 at 15:17:33 +0100, Michal Privoznik wrote: So far, this is pure code replacement. But once we introduce reference counting to virNetworkObj this will be more handy as there'll be only one function to change: virNetworkObjEndAPI(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/network/bridge_driver.c | 57 +++-- 1 file changed, 19 insertions(+), 38 deletions(-) ACK signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 22/31] virNetworkObjList: Derive from virObjectLockableClass
On Thu, Feb 26, 2015 at 15:17:31 +0100, Michal Privoznik wrote: Later we can turn APIs to lock the object if needed instead of relying on caller to mutually exclude itself (probably done by locking a big lock anyway). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/network_conf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) This patch is short enough to be squashed into patch 27 that will actually use it. 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] storage: tweak condition to properly test lseek
On 03/03/2015 08:57 AM, Erik Skultety wrote:
According to the POSIX standard, off_t (returned by lseek) is defined as
signed integral type no shorter than int. Because our offset variable is
defined
as unsigned long long, the original check was passed successfully if
UINT64_MAX had
been used as offset value, due to implicit conversion.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177219
---
src/fdstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/src/fdstream.c b/src/fdstream.c
index 5d80fc2..b8ea86e 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -610,7 +610,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
}
if (offset
-lseek(fd, offset, SEEK_SET) != offset) {
+lseek(fd, offset, SEEK_SET) 0) {
virReportSystemError(errno,
_(Unable to seek %s to %llu),
path, offset);
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/10] parallels: set cpu mode when applying xml configuration
From: Mikhail Feoktistov mfeoktis...@parallels.com
Otherwise exporting existing domain config and defining a new one like this:
virsh -c parallels:///system dumpxml instance01 my.xml
virsh -c parallels:///system define my.xml
leads to an error because PCS default x64 mode turns to x32.
Thus, we need to set correct cpuMode in prlsdkDoApplyConfig() explicitly.
Signed-off-by: Mikhail Feoktistov mfeoktis...@parallels.com
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index fec145d..9a2d658 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2869,6 +2869,19 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
pret = PrlVmCfg_SetCpuCount(sdkdom, def-vcpus);
prlsdkCheckRetGoto(pret, error);
+switch (def-os.arch) {
+case VIR_ARCH_X86_64:
+pret = PrlVmCfg_SetCpuMode(sdkdom, PCM_CPU_MODE_64);
+break;
+case VIR_ARCH_I686:
+pret = PrlVmCfg_SetCpuMode(sdkdom, PCM_CPU_MODE_32);
+break;
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown CPU mode: %X),
def-os.arch);
+goto error;
+}
+prlsdkCheckRetGoto(pret, error);
+
if (prlsdkClearDevices(sdkdom) 0)
goto error;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/10] parallels: introduce and use string constants for network types and names
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_network.c |4 ++--
src/parallels/parallels_sdk.c |6 +++---
src/parallels/parallels_utils.h |8 +++-
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 3e7087d..2c793c1 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -211,12 +211,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
goto cleanup;
}
-if (STREQ(tmp, bridged)) {
+if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_BRIDGE;
if (parallelsGetBridgedNetInfo(def, jobj) 0)
goto cleanup;
-} else if (STREQ(tmp, host-only)) {
+} else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_NONE;
if (parallelsGetHostOnlyNetInfo(def, def-name) 0)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 0c9837a..83af020 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -709,7 +709,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
* always up */
net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
if (VIR_STRDUP(net-data.network.name,
- PARALLELS_ROUTED_NETWORK_NAME) 0)
+ PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0)
goto cleanup;
return 0;
}
@@ -728,7 +728,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
if (emulatedType == PNA_ROUTED) {
if (VIR_STRDUP(net-data.network.name,
- PARALLELS_ROUTED_NETWORK_NAME) 0)
+ PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0)
goto cleanup;
} else {
pret = PrlVmDevNet_GetVirtualNetworkId(netAdapter, NULL, buflen);
@@ -2649,8 +2649,8 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
virDomainNetDefPtr net)
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
-if (STREQ(net-data.network.name, PARALLELS_ROUTED_NETWORK_NAME)) {
pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
+if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
prlsdkCheckRetGoto(pret, cleanup);
} else {
pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net-data.network.name);
diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
index bebf841..63fe78f 100644
--- a/src/parallels/parallels_utils.h
+++ b/src/parallels/parallels_utils.h
@@ -47,7 +47,13 @@
_(no domain with matching uuid '%s'), uuidstr); \
} while (0)
-# define PARALLELS_ROUTED_NETWORK_NAME Routed
+# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME Routed
+# define PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME Bridged
+
+# define PARALLELS_REQUIRED_HOSTONLY_NETWORK Host-Only
+# define PARALLELS_HOSTONLY_NETWORK_TYPE host-only
+# define PARALLELS_REQUIRED_BRIDGED_NETWORK Bridged
+# define PARALLELS_BRIDGED_NETWORK_TYPE bridged
struct _parallelsConn {
virMutex lock;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/10] parallels: fix home directory for VMs
Failures of parallelsStorageOpen occured because we incorrectly treated
path to VM' configuration file as a directory. Now initialization of
parallels VM domains home directory is fixed.
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 9a2d658..e0c5895 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1247,6 +1247,14 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
pret = PrlVmCfg_GetHomePath(sdkdom, pdom-home, buflen);
prlsdkCheckRetGoto(pret, error);
+/* For VMs pdom-home is actually /directory/config.pvs */
+if (!IS_CT(def)) {
+/* Get rid of /config.pvs in path string */
+char *s = strrchr(pdom-home, '/');
+if (s)
+*s = '\0';
+}
+
if (olddom) {
/* assign new virDomainDef without any checks */
/* we can't use virDomainObjAssignDef, because it checks
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/10] parallels: better bridge network interface support
In order to support 'bridge' network adapters in parallels
driver we need to plug our veth devices into corresponding
linux bridges.
We are going to do this by reusing our abstraction of
Virtual Networks in terms of PCS. On a domain creation, we
create a new Virtual Network naming it with the same name
as a source bridge for each network interface.
Having done this, we plug PCS veth interfaces created with names of
target dev into specified bridges using our standard PCS procedures
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c | 101 +++--
1 files changed, 87 insertions(+), 14 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 83af020..cf81540 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -692,9 +692,6 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
/* use device name, shown by prlctl as target device
* for identifying network adapter in virDomainDefineXML */
-pret = PrlVmDev_GetIndex(netAdapter, netAdapterIndex);
-prlsdkCheckRetGoto(pret, cleanup);
-
pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, NULL, buflen);
prlsdkCheckRetGoto(pret, cleanup);
@@ -704,6 +701,9 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net-ifname, buflen);
prlsdkCheckRetGoto(pret, cleanup);
+pret = PrlVmDev_GetIndex(netAdapter, netAdapterIndex);
+prlsdkCheckRetGoto(pret, cleanup);
+
if (isCt netAdapterIndex == (PRL_UINT32) -1) {
/* venet devices don't have mac address and
* always up */
@@ -741,6 +741,16 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
net-data.network.name,
buflen);
prlsdkCheckRetGoto(pret, cleanup);
+
+/*
+ * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters
+ * except those whose Virtual Network Id differ from Parallels
+ * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME
+ * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME
+ */
+if (!STREQ(net-data.network.name,
PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME))
+net-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+
}
pret = PrlVmDev_IsConnected(netAdapter, isConnected);
@@ -2621,10 +2631,12 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac,
char *macstr)
return macstr;
}
-static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net)
+static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn,
virDomainNetDefPtr net)
{
PRL_RESULT pret;
PRL_HANDLE sdknet = PRL_INVALID_HANDLE;
+PRL_HANDLE vnet = PRL_INVALID_HANDLE;
+PRL_HANDLE job = PRL_INVALID_HANDLE;
int ret = -1;
char macstr[PRL_MAC_STRING_BUFNAME];
@@ -2649,10 +2661,39 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
virDomainNetDefPtr net)
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
-pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
-if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
+if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+if (STREQ(net-data.network.name,
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
+prlsdkCheckRetGoto(pret, cleanup);
+} else if (STREQ(net-data.network.name,
PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmDevNet_SetVirtualNetworkId(sdknet,
net-data.network.name);
+prlsdkCheckRetGoto(pret, cleanup);
+}
+} else if (net-type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+/*
+ * For this type of adapter we create a new
+ * Virtual Network assuming that bridge with given name exists
+ * Failing creating this means domain creation failure
+ */
+pret = PrlVirtNet_Create(vnet);
prlsdkCheckRetGoto(pret, cleanup);
-} else {
+
+pret = PrlVirtNet_SetNetworkId(vnet, net-data.network.name);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET);
+prlsdkCheckRetGoto(pret, cleanup);
+
+job = PrlSrv_AddVirtualNetwork(privconn-server, vnet, 0);
+if (PRL_FAILED(pret = waitJob(job, privconn-jobTimeout)))
+goto cleanup;
+
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
+prlsdkCheckRetGoto(pret, cleanup);
+
pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net-data.network.name);
prlsdkCheckRetGoto(pret, cleanup);
}
@@ -2665,10
[libvirt] [PATCH 02/10] parallels: don't forget to unlock domain if unregister fails
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_driver.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index c9338b5..90b8b0a 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -938,6 +938,7 @@ parallelsDomainUndefineFlags(virDomainPtr domain,
{
parallelsConnPtr privconn = domain-conn-privateData;
virDomainObjPtr dom = NULL;
+int ret;
virCheckFlags(0, -1);
@@ -947,7 +948,11 @@ parallelsDomainUndefineFlags(virDomainPtr domain,
return -1;
}
-return prlsdkUnregisterDomain(privconn, dom);
+ret = prlsdkUnregisterDomain(privconn, dom);
+if (ret)
+ virObjectUnlock(dom);
+
+return ret;
}
static int
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/10] parallels: set network adapter device status to connected
when a new network adapter device is added
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index cf81540..d4e19e7 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2649,7 +2649,7 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
parallelsConnPtr privconn, virDomainN
pret = PrlVmDev_SetEnabled(sdknet, 1);
prlsdkCheckRetGoto(pret, cleanup);
-pret = PrlVmDev_SetConnected(sdknet, net-linkstate);
+pret = PrlVmDev_SetConnected(sdknet, 1);
prlsdkCheckRetGoto(pret, cleanup);
if (net-ifname) {
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/10] parallels: switch off offline management feature
which is on by default when a new VM/CT is created. We should do this because this feature can't be controlled by libvirt now and it sets up some iptables rules. So it's better to do this to avoid potential conflict of different set of rules or to avoid unexpected behavior. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index ef513d7..aa26cff 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -3061,6 +3061,9 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetDefaultConfig(sdkdom, srvconf, PVS_GUEST_VER_LIN_REDHAT, 0); prlsdkCheckRetGoto(pret, cleanup); +pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); +prlsdkCheckRetGoto(pret, cleanup); + ret = prlsdkDoApplyConfig(conn, sdkdom, def); if (ret) goto cleanup; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/10] parallels: fix parallelsLoadNetworks
Don't fail initialization of parallels driver if
parallelsLoadNetwork fails for optional networks.
This can happen when some of them are added manually
and configured incompletely. PCS requires only two networks
created automatically (named Host-Only and Bridged), others
are optional and their incompletenes can be ignored.
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_network.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 2c793c1..45d1e4c 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -180,9 +180,10 @@ static int parallelsGetHostOnlyNetInfo(virNetworkDefPtr
def, const char *name)
return ret;
}
-static virNetworkObjPtr
+static int
parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
{
+int ret = -1;
virNetworkObjPtr net;
virNetworkDefPtr def;
const char *tmp;
@@ -214,13 +215,25 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_BRIDGE;
-if (parallelsGetBridgedNetInfo(def, jobj) 0)
+if (parallelsGetBridgedNetInfo(def, jobj) 0) {
+
+/* Only mandatory networks required to be configured completely */
+if (!STREQ(def-name, PARALLELS_REQUIRED_BRIDGED_NETWORK))
+ret = 0;
+
goto cleanup;
+}
} else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_NONE;
-if (parallelsGetHostOnlyNetInfo(def, def-name) 0)
+if (parallelsGetHostOnlyNetInfo(def, def-name) 0) {
+
+/* Only mandatory networks required to be configured completely */
+if (!STREQ(def-name, PARALLELS_REQUIRED_HOSTONLY_NETWORK))
+ret = 0;
+
goto cleanup;
+}
} else {
parallelsParseError();
goto cleanup;
@@ -231,11 +244,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
net-active = 1;
net-autostart = 1;
virNetworkObjUnlock(net);
-return net;
+ret = 0;
+return ret;
cleanup:
virNetworkDefFree(def);
-return NULL;
+return ret;
}
static virNetworkObjPtr
@@ -277,7 +291,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
static int parallelsLoadNetworks(parallelsConnPtr privconn)
{
virJSONValuePtr jobj, jobj2;
-virNetworkObjPtr net;
int ret = -1;
int count;
size_t i;
@@ -302,8 +315,8 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
goto cleanup;
}
-net = parallelsLoadNetwork(privconn, jobj2);
-if (!net)
+ret = parallelsLoadNetwork(privconn, jobj2);
+if (!ret)
goto cleanup;
}
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/10] parallels: minor cleanup
indentation is fixed, unnecessary error message removed,
unnecessary job freeing removed
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index e0c5895..0c9837a 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -702,7 +702,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
goto cleanup;
pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net-ifname, buflen);
-prlsdkCheckRetGoto(pret, cleanup);
+prlsdkCheckRetGoto(pret, cleanup);
if (isCt netAdapterIndex == (PRL_UINT32) -1) {
/* venet devices don't have mac address and
@@ -1352,7 +1352,6 @@ prlsdkLoadDomains(parallelsConnPtr privconn)
error:
PrlHandle_Free(result);
-PrlHandle_Free(job);
return -1;
}
@@ -1732,8 +1731,6 @@ prlsdkDomainChangeState(virDomainPtr domain,
pdom = dom-privateData;
pret = chstate(privconn, pdom-sdkdom);
-virReportError(VIR_ERR_OPERATION_FAILED,
- _(Can't change domain state: %d), pret);
if (PRL_FAILED(pret)) {
virResetLastError();
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/10] parallels: make E1000 network adapter type default
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index d4e19e7..ef513d7 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2661,6 +2661,10 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
parallelsConnPtr privconn, virDomainN
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
+/* Other alternatives: PNT_VIRTIO, PNT_RTL */
+pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000);
+prlsdkCheckRetGoto(pret, cleanup);
+
if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (STREQ(net-data.network.name,
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 00/10] Network enhancements and other fixes
Maxim Nestratov (9): parallels: don't forget to unlock domain if unregister fails parallels: fix home directory for VMs parallels: minor cleanup parallels: introduce and use string constants for network types and names parallels: fix parallelsLoadNetworks parallels: better bridge network interface support parallels: set network adapter device status to connected parallels: make E1000 network adapter type default parallels: switch off offline management feature Mikhail Feoktistov (1): parallels: set cpu mode when applying xml configuration -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] libxl: a few minor patches
This series contains a few simple changes to the libxl driver. Patches 1 and 2 were included in https://www.redhat.com/archives/libvir-list/2015-February/msg00611.html To make an upcoming V2 of that series easier to review, I removed the mostly unrelated patches and include them here, along with one other small fix. Jim Fehlig (3): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: remove unneeded cleanup_unlock label src/libxl/libxl_domain.c | 13 ++--- src/libxl/libxl_driver.c | 10 +++--- 2 files changed, 5 insertions(+), 18 deletions(-) -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] libxl: remove redundant calls to libxl_evdisable_domain_death
Domain death watch is already disabled in libxlDomainCleanup. No
need to disable it a second and third time.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/libxl/libxl_domain.c | 8
1 file changed, 8 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 21c41d7..e186c53 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -439,9 +439,6 @@ libxlDomainObjPrivateDispose(void *obj)
{
libxlDomainObjPrivatePtr priv = obj;
-if (priv-deathW)
-libxl_evdisable_domain_death(priv-ctx, priv-deathW);
-
libxlDomainObjFreeJob(priv);
virChrdevFree(priv-devs);
libxl_ctx_free(priv-ctx);
@@ -456,11 +453,6 @@ libxlDomainObjPrivateFree(void *data)
{
libxlDomainObjPrivatePtr priv = data;
-if (priv-deathW) {
-libxl_evdisable_domain_death(priv-ctx, priv-deathW);
-priv-deathW = NULL;
-}
-
virObjectUnref(priv);
}
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] libxl: remove unneeded cleanup_unlock label
In the old days of a global driver lock, it was necessary to unlock the driver after a domain restore operation. When the global lock was removed from the driver, some remnants were left behind in libxlDomainRestoreFlags. Remove this unneeded (and incorrect) code. Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/libxl/libxl_driver.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ce3a99b..55fa64f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1464,17 +1464,17 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, fd = libxlDomainSaveImageOpen(driver, cfg, from, def, hdr); if (fd 0) -goto cleanup_unlock; +return -1; if (virDomainRestoreFlagsEnsureACL(conn, def) 0) -goto cleanup_unlock; +return -1; if (!(vm = virDomainObjListAdd(driver-domains, def, driver-xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) -goto cleanup_unlock; +goto cleanup; def = NULL; @@ -1492,10 +1492,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, virObjectUnlock(vm); virObjectUnref(cfg); return ret; - - cleanup_unlock: -libxlDriverUnlock(driver); -goto cleanup; } static int -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] libxl: use libxl_ctx passed to libxlConsoleCallback
Instead of using the libxl_ctx in the libxlDomainObjPrivatePtr,
use the ctx passed to the callback.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/libxl/libxl_domain.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e186c53..9af5758 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1158,10 +1158,9 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv,
libxl_domain_config *d_config)
}
static void
-libxlConsoleCallback(libxl_ctx *ctx, libxl_event* ev, void *for_callback)
+libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
{
virDomainObjPtr vm = for_callback;
-libxlDomainObjPrivatePtr priv = vm-privateData;
size_t i;
virObjectLock(vm);
@@ -1175,7 +1174,7 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event* ev,
void *for_callback)
console_type =
(chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
-ret = libxl_console_get_tty(priv-ctx, ev-domid,
+ret = libxl_console_get_tty(ctx, ev-domid,
chr-target.port, console_type,
console);
if (!ret) {
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] util: don't fail if no PortData is found while getting migrateData
---
V1 here:
https://www.redhat.com/archives/libvir-list/2015-February/msg00388.html
V2:
Add --if-exists option to ovs-vsctl cmd, making ovs-vsctl not raise error
if there's no portData
available. Suggested by Martin.
We Tested the patch, it works.
---
Introduced by f6a2f97e
Problem description:
After multiple times of migrating a domain, which has an ovs interface with no
portData set,
with non-shared disk, nbd ports got overflowed.
The steps to reproduce the problem:
1 define and start a domain with its network configured as:
interface type='bridge'
source bridge='br0'/
virtualport type='openvswitch'
/virtualport
model type='virtio'/
driver name='vhost' queues='4'/
/interface
2 do not set the network's portData.
3 migrate(ToURI2) it with flag 91(1011011), which means:
VIR_MIGRATE_LIVE
VIR_MIGRATE_PEER2PEER
VIR_MIGRATE_PERSIST_DEST
VIR_MIGRATE_UNDEFINE_SOURCE
VIR_MIGRATE_NON_SHARED_DISK
4 migrate success, but we got an error log in libvirtd.log:
error : virCommandWait:2423 : internal error: Child process (ovs-vsctl
--timeout=5 get Interface
vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key
PortData in Interface
record vnet1 column external_ids
5 migrate it back, migrate it , migrate it back, ...
6 nbd port got overflowed.
The reasons for the problem is :
1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is
available for the ovs
interface of a domain. (We think it's not appropriate, as portData is just
OPTIONAL)
2 in func qemuMigrationBakeCookie(), it fails in
qemuMigrationCookieAddNetwork(), and returns with -1.
qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is still
NULL.
3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails,
migration still successes.
cookie is NULL, it's not baked on the src side.
4 On the destination side, it would alloc a port first and then free the nbd
port in COOKIE.
But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src
side. thus the nbd port
is not freed.
In this patch, we add --if-exists option to make ovs-vsctl not raise error if
there's no portData available.
Further more, because portData may be NULL in the cookie at the dest side,
check it before setting portData.
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/util/virnetdevopenvswitch.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index e5c87bb..722d0dd 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -30,9 +30,12 @@
#include virerror.h
#include virmacaddr.h
#include virstring.h
+#include virlog.h
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_LOG_INIT(util.netdevopenvswitch);
+
/**
* virNetDevOpenvswitchAddPort:
* @brname: the bridge name
@@ -206,7 +209,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate,
const char *ifname)
virCommandPtr cmd = NULL;
int ret = -1;
-cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface,
+cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, --if-exists, get,
Interface,
ifname, external_ids:PortData, NULL);
virCommandSetOutputBuffer(cmd, migrate);
@@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate,
const char *ifname)
virCommandPtr cmd = NULL;
int ret = -1;
+if (!migrate) {
+VIR_DEBUG(No port data for interface %s, ifname);
+return 0;
+}
+
cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, set,
Interface, ifname, NULL);
virCommandAddArgFormat(cmd, external_ids:PortData=%s, migrate);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix not remove the pidfile when close a vm after restart libvirtd
On 03/03/2015 06:57 PM, Michal Privoznik wrote:
On 02.03.2015 10:37, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1197600
when create a happy vm and then restart libvirtd, we will loss
priv-pidfile, because we don't check if it is there is a pidfile.
However we only use this pidfile when we start the vm, and won't use
it after it start, so this is not a big deal.
But it is strange when vm is offline but pidfile still exist, so
remove vmname.pid in state dir (maybe /run/libvirt/qemu/)when
priv-pidfile is NULL.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_process.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 515402e..46b93b3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -92,6 +92,7 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
{
char ebuf[1024];
char *file = NULL;
+char *pidfile = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = -1;
@@ -99,16 +100,19 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
if (virAsprintf(file, %s/%s.xml, cfg-stateDir, vm-def-name) 0)
goto cleanup;
+if (virAsprintf(pidfile, %s/%s.pid, cfg-stateDir, vm-def-name) 0)
+goto cleanup;
If this fails, @file is leaked. And there's a helper function to
generate pid file path: virPidFileBuildPath(). I know it does exactly
the same, but lets use that, so whenever we decide on changing the path,
we need to change it at one place only, instead of digging through
source code just to check if somebody has not used virAsprintf() directly.
Yes, i forgot check if file will be leaked, thanks for pointing out that.
About the virPidFileBuildPath(), i should say i missed this function
from the code :) and use virPidFileBuildPath() is better than
virAsprintf() in every sense.
+
if (unlink(file) 0 errno != ENOENT errno != ENOTDIR)
VIR_WARN(Failed to remove domain XML for %s: %s,
vm-def-name, virStrerror(errno, ebuf, sizeof(ebuf)));
VIR_FREE(file);
-if (priv-pidfile
-unlink(priv-pidfile) 0
+if (unlink(priv-pidfile ? priv-pidfile : pidfile) 0
errno != ENOENT)
VIR_WARN(Failed to remove PID file for %s: %s,
vm-def-name, virStrerror(errno, ebuf, sizeof(ebuf)));
+VIR_FREE(pidfile);
ret = 0;
cleanup:
While this works, I think we need a different approach:
https://www.redhat.com/archives/libvir-list/2015-March/msg00047.html
Good approach :)
Michal
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 26/31] parallels_network: Use virNetworkObjEndAPI
On Thu, Feb 26, 2015 at 15:17:35 +0100, Michal Privoznik wrote: So far, this is pure code replacement. But once we introduce reference counting to virNetworkObj this will be more handy as there'll be only one function to change: virNetworkObjEndAPI(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/parallels/parallels_network.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) ACK, I see that you will fix the two remaining places in parallelsLoadNetwork() and parallelsAddRoutedNetwork() in one of the next patches. 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 v1 21/31] network_conf: Make virNetworkObj actually virObject
On 03.03.2015 14:10, Peter Krempa wrote:
On Tue, Mar 03, 2015 at 13:47:32 +0100, Michal Privoznik wrote:
On 03.03.2015 12:05, Peter Krempa wrote:
On Thu, Feb 26, 2015 at 15:17:30 +0100, Michal Privoznik wrote:
So far it's just a structure which happens to have 'Obj' in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
cfg.mk| 2 -
src/conf/network_conf.c | 126
--
src/conf/network_conf.h | 8 +--
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56 -
src/parallels/parallels_network.c | 16 ++---
src/test/test_driver.c| 32 +-
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml| 2 -
9 files changed, 125 insertions(+), 125 deletions(-)
...
@@ -602,17 +620,17 @@ void virNetworkRemoveInactive(virNetworkObjListPtr
nets,
{
size_t i;
-virNetworkObjUnlock(net);
+virObjectUnlock(net);
for (i = 0; i nets-count; i++) {
-virNetworkObjLock(nets-objs[i]);
+virObjectLock(nets-objs[i]);
if (nets-objs[i] == net) {
-virNetworkObjUnlock(nets-objs[i]);
-virNetworkObjFree(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
+virObjectUnref(nets-objs[i]);
VIR_DELETE_ELEMENT(nets-objs, i, nets-count);
break;
}
-virNetworkObjUnlock(nets-objs[i]);
+virObjectUnlock(nets-objs[i]);
}
}
The function above is very strange. Your changes should not have any
functional impact, but I think we should fix it right away.
1) why do we unlock the object that is going to be deleted ?
To get the order of locking right.
Well, this would make it only simpler to deal with locking in the loop
below since the code always grabs the network driver lock for every
operation. And since ...
2) why do we bother locking the objects in between? We are comparing
just the pointer to the object so there's no need to wait for the lock.
Yep, I've saved that for a separate patch, which is not posted yet though.
... locking of the object doesn't make sense there shouldn't be any
issue.
If we keep the original object locked there's no need to do any of that.
3) the network object is freed _before_ it's removed from the list.
Although the list is always locked before access, I don't think that's a
good practice.
Not anymore. Any API that enters the remove function already has at
least one reference (obtained via virNetworkObjFind*). So this merely
decrease the refcount on the object since the list does no longer hold a
reference to the object.
That is not true at this point. At this point in the series there's only
one reference ever in the list so that the object gets deleted in this
function. You add reference counting in patch 28/31.
Okay, considered this squashed in:
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 8f140d2..3ea8975 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -620,17 +620,13 @@ void virNetworkRemoveInactive(virNetworkObjListPtr
nets,
{
size_t i;
-virObjectUnlock(net);
for (i = 0; i nets-count; i++) {
-virObjectLock(nets-objs[i]);
if (nets-objs[i] == net) {
-virObjectUnlock(nets-objs[i]);
-virObjectUnref(nets-objs[i]);
-
VIR_DELETE_ELEMENT(nets-objs, i, nets-count);
+virObjectUnlock(net);
+virObjectUnref(net);
break;
}
-virObjectUnlock(nets-objs[i]);
}
}
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 24/31] bridge_driver: Use virNetworkObjEndAPI
On Thu, Feb 26, 2015 at 15:17:33 +0100, Michal Privoznik wrote: So far, this is pure code replacement. But once we introduce reference counting to virNetworkObj this will be more handy as there'll be only one function to change: virNetworkObjEndAPI(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/network/bridge_driver.c | 57 +++-- 1 file changed, 19 insertions(+), 38 deletions(-) In the qemu driver that is the inspiration for this change we always pair the *EndAPI call with a *ObjFrom* ... in this case networkObjFromNetwork. I'd really appreciate if you could first scrub the code base of direct lookup and use the networkObjFromNetwork helper instead so that we can pair every EndAPI call with that. ACK to this patch though. If you don't fancy cleaning the existing places that manually lookup the object I can do it. Please let me know. 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 v1 16/31] bridge_driver: Adapt to new virNetworkObjList accessors
On Thu, Feb 26, 2015 at 15:17:25 +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/network/bridge_driver.c | 333
1 file changed, 148 insertions(+), 185 deletions(-)
Some bikeshedding below ...
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 268af49..1c73342 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -342,105 +342,91 @@ networkBridgeDummyNicName(const char *brname)
return nicname;
}
-/* Update the internal status of all allegedly active networks
- * according to external conditions on the host (i.e. anything that
- * isn't stored directly in each network's state file). */
-static void
-networkUpdateAllState(void)
+static int
+networkUpdateAllState(virNetworkObjPtr obj,
This function now updates state of one network, thus the 'All' word is
misleading.
+ void *opaque ATTRIBUTE_UNUSED)
{
-size_t i;
+int ret = -1;
-for (i = 0; i driver-networks-count; i++) {
-virNetworkObjPtr obj = driver-networks-objs[i];
+virNetworkObjLock(obj);
+if (!virNetworkObjIsActive(obj)) {
+virNetworkObjUnlock(obj);
+return 0;
+}
-virNetworkObjLock(obj);
-if (!virNetworkObjIsActive(obj)) {
-virNetworkObjUnlock(obj);
-continue;
-}
+switch (obj-def-forward.type) {
+case VIR_NETWORK_FORWARD_NONE:
+case VIR_NETWORK_FORWARD_NAT:
+case VIR_NETWORK_FORWARD_ROUTE:
+/* If bridge doesn't exist, then mark it inactive */
+if (!(obj-def-bridge virNetDevExists(obj-def-bridge) == 1))
+obj-active = 0;
+break;
-switch (obj-def-forward.type) {
-case VIR_NETWORK_FORWARD_NONE:
-case VIR_NETWORK_FORWARD_NAT:
-case VIR_NETWORK_FORWARD_ROUTE:
-/* If bridge doesn't exist, then mark it inactive */
-if (!(obj-def-bridge virNetDevExists(obj-def-bridge) ==
1))
+case VIR_NETWORK_FORWARD_BRIDGE:
+if (obj-def-bridge) {
+if (virNetDevExists(obj-def-bridge) != 1)
obj-active = 0;
break;
-
-case VIR_NETWORK_FORWARD_BRIDGE:
-if (obj-def-bridge) {
-if (virNetDevExists(obj-def-bridge) != 1)
-obj-active = 0;
-break;
-}
-/* intentionally drop through to common case for all
- * macvtap networks (forward='bridge' with no bridge
- * device defined is macvtap using its 'bridge' mode)
- */
-case VIR_NETWORK_FORWARD_PRIVATE:
-case VIR_NETWORK_FORWARD_VEPA:
-case VIR_NETWORK_FORWARD_PASSTHROUGH:
-/* so far no extra checks */
-break;
-
-case VIR_NETWORK_FORWARD_HOSTDEV:
-/* so far no extra checks */
-break;
-}
-
-/* Try and read dnsmasq/radvd pids of active networks */
-if (obj-active obj-def-ips (obj-def-nips 0)) {
-char *radvdpidbase;
-
-ignore_value(virPidFileReadIfAlive(driver-pidDir,
- obj-def-name,
- obj-dnsmasqPid,
-
dnsmasqCapsGetBinaryPath(driver-dnsmasqCaps)));
-radvdpidbase = networkRadvdPidfileBasename(obj-def-name);
-if (!radvdpidbase)
-break;
-ignore_value(virPidFileReadIfAlive(driver-pidDir,
- radvdpidbase,
- obj-radvdPid, RADVD));
-VIR_FREE(radvdpidbase);
}
-
-virNetworkObjUnlock(obj);
+/* intentionally drop through to common case for all
+ * macvtap networks (forward='bridge' with no bridge
+ * device defined is macvtap using its 'bridge' mode)
+ */
+case VIR_NETWORK_FORWARD_PRIVATE:
+case VIR_NETWORK_FORWARD_VEPA:
+case VIR_NETWORK_FORWARD_PASSTHROUGH:
+/* so far no extra checks */
+break;
+
+case VIR_NETWORK_FORWARD_HOSTDEV:
+/* so far no extra checks */
+break;
}
-/* remove inactive transient networks */
-i = 0;
-while (i driver-networks-count) {
-virNetworkObjPtr obj = driver-networks-objs[i];
-virNetworkObjLock(obj);
-
-if (!obj-persistent !obj-active) {
-networkRemoveInactive(obj);
-continue;
-}
+/* Try and read dnsmasq/radvd pids of active networks */
+if (obj-active obj-def-ips (obj-def-nips 0)) {
+char *radvdpidbase;
+
+ignore_value(virPidFileReadIfAlive(driver-pidDir,
+ obj-def-name,
+
Re: [libvirt] [PATCH v1 20/31] network_conf: Turn struct _virNetworkObjList private
On Thu, Feb 26, 2015 at 15:17:29 +0100, Michal Privoznik wrote: Now that all the code uses accessors, don't expose the structure anyway. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/network_conf.c | 7 +++ src/conf/network_conf.h | 6 -- 2 files changed, 7 insertions(+), 6 deletions(-) ACK, 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] qemuProcessReconnect: Fill in pid file path
On Tue, Mar 03, 2015 at 11:56:37AM +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1197600
So, libvirt uses pid file to track pid of started qemus. Whenever
a domain is started, its pid is put into corresponding pid file.
The pid file path is generated based on domain name and stored
into domain object internals. However, it's not stored in the
status XML and therefore lost on daemon restarts. Hence, later,
when domain is being shut down, the daemon does not know which
pid file to unlink, and the correct pid file is left behind. To
avoid this, lets generate the pid file path again in
qemuProcessReconnect().
Reported-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_process.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7ce6115..e77616a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3762,6 +3762,13 @@ qemuProcessReconnect(void *opaque)
cfg = virQEMUDriverGetConfig(driver);
priv = obj-privateData;
+/* XXX If we ever gonna change pid file pattern, come up with
+ * some intelligence here to deal with old paths. */
We already have arbitrary patterns due to virDomainQEMUAttach() which can
talk to an existing QEMU. We should save the pidfile pattern in the status
XML file so we remember it. For upgrades from existing deployments we would
have to cope with the status XML not having pidfile, and so fallback to
building it again.
+if (!(priv-pidfile = virPidFileBuildPath(cfg-stateDir,
obj-def-name))) {
+virReportSystemError(errno, %s, _(Failed to build pidfile
path.));
+goto killvm;
+}
+
if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) 0)
goto killvm;
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 v3 0/9] qemu: Add quorum support to libvirt
On Thu, Feb 26, 2015 at 5:04 PM, Peter Krempa pkre...@redhat.com wrote: On Mon, Feb 23, 2015 at 14:18:31 +0100, Matthias Gatto wrote: On Tue, Feb 10, 2015 at 4:43 PM, Matthias Gatto matthias.ga...@outscale.com wrote: The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Add nodename We need to add nodename support in _virStorageSource because qemu use them for their child. 5) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 6) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add when only attaching quorum (but i'm pretty sure this solution is not the good one) V2: -Rebase on master -Add Documentation V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size. Matthias Gatto (9): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name docs/formatdomain.html.in | 27 - docs/schemas/domaincommon.rng | 96 +++-- docs/schemas/storagecommon.rng| 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c| 195 ++ src/conf/storage_conf.c | 23 ++-- src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c| 4 +- src/qemu/qemu_command.c | 114 src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 30 +++--- src/qemu/qemu_migration.c | 1 + src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 33 +++--- src/storage/storage_backend_fs.c | 36 --- src/storage/storage_backend_gluster.c | 10 +- src/storage/storage_backend_logical.c | 15 ++- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 116 +--- src/util/virstoragefile.h | 12 ++- tests/virstoragetest.c| 18 ++-- 23 files changed, 573 insertions(+), 173 deletions(-) -- 1.8.3.1 ping Sorry for taking so long. I was pretty busy in the last time and need to switch context to the storage part of libvirt again at first. I have this series on the radar and will try to review it once I get rid of some stuff. Peter ok, Thank you -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] PowerPC : Miscellaneous fixes for 'ppc64le' architecture.
On 26.02.2015 18:09, Prerna Saxena wrote: From a28ef5a3e7b9cb023948cf97d9f472bb3a1e06d3 Mon Sep 17 00:00:00 2001 From: Prerna Saxena pre...@linux.vnet.ibm.com Date: Thu, 26 Feb 2015 22:31:05 +0530 This series adds few miscellaneous fixes for PowerPC 64-bit Little Endian Architecture. Changelog : == v1 of Patch 1/2 already posted and acked : https://www.redhat.com/archives/libvir-list/2015-February/msg00486.html Patch 2/2 adds a testcase to supplement patch 1. Prerna Saxena (2): PowerPC: Augment XML schema to include 'ppc64le' arch and newer pseries-2.* machine types. Tests : Add test for 'ppc64le' architecture. docs/schemas/basictypes.rng| 1 + docs/schemas/domaincommon.rng | 7 - .../qemuxml2argv-pseries-cpu-le.args | 7 + .../qemuxml2argv-pseries-cpu-le.xml| 17 tests/qemuxml2argvtest.c | 2 ++ tests/testutilsqemu.c | 30 ++ 6 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml I've tweaked the commit messages a bit, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] Add test for virtio serial port assignment
Add a test to demonstrate the effect of this series. --- .../qemuxml2argv-channel-virtio-autoassign.args| 20 + .../qemuxml2argv-channel-virtio-autoassign.xml | 50 ++ tests/qemuxml2argvtest.c | 2 + 3 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args new file mode 100644 index 000..d64a228 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args @@ -0,0 +1,20 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ +-device virtio-serial-pci,id=virtio-serial0,max_ports=4,vectors=4,bus=pci.0\ +,addr=0x3 -device virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa \ +-usb -hda /dev/HostVG/QEMUGuest1 \ +-chardev pty,id=charchannel0 -device virtserialport,bus=virtio-serial0.0,nr=1,\ +chardev=charchannel0,id=channel0,name=org.linux-kvm.port.0 \ +-chardev pty,id=charchannel1 -device virtserialport,bus=virtio-serial0.0,nr=2,\ +chardev=charchannel1,id=channel1,name=org.linux-kvm.port.foo \ +-chardev pty,id=charchannel2 -device virtserialport,bus=virtio-serial0.0,nr=1,\ +chardev=charchannel2,id=channel2,name=org.linux-kvm.port.bar \ +-chardev pty,id=charchannel3 -device virtserialport,bus=virtio-serial0.2,nr=1,\ +chardev=charchannel3,id=channel3,name=org.linux-kvm.port.wizz \ +-chardev pty,id=charchannel4 -device virtserialport,bus=virtio-serial0.0,nr=3,\ +chardev=charchannel4,id=channel4,name=org.linux-kvm.port.ooh \ +-chardev pty,id=charchannel5 -device virtserialport,bus=virtio-serial0.0,nr=4,\ +chardev=charchannel5,id=channel5,name=org.linux-kvm.port.lla \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml new file mode 100644 index 000..ac0744e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml @@ -0,0 +1,50 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='virtio-serial' index='0' ports='4' vectors='4'/ +controller type='virtio-serial' index='1' + address type='pci' domain='0x' bus='0x00' slot='0x0a' function='0x0'/ +/controller +channel type='pty' + target type='virtio' name='org.linux-kvm.port.0'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.foo'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.bar'/ + address type='virtio-serial' controller='0' port='1'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.wizz'/ + address type='virtio-serial' controller='0' bus='2'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.ooh'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.lla'/ +/channel +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 681f8fe..41cadb6 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1064,6 +1064,8 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST(channel-virtio-auto, QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); +DO_TEST(channel-virtio-autoassign, +QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST(console-virtio, QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST(console-virtio-many, -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Allocate virtio-serial addresses
Instead of simply incrementing the port, respect maximum port values and use multiple controllers. Ján Tomko (5): Add test for virtio serial port assignment Add functions to track virtio-serial addresses Allocate virtio-serial addresses when starting a domain Expand the address set when attaching a virtio-serial controller Assign an address when hotplugging a virtio-serial device src/conf/domain_addr.c | 382 + src/conf/domain_addr.h | 45 +++ src/conf/domain_conf.c | 29 -- src/libvirt_private.syms | 8 + src/qemu/qemu_command.c| 63 src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_hotplug.c| 32 +- src/qemu/qemu_process.c| 2 + tests/qemuhotplugtest.c| 2 +- .../qemuxml2argv-channel-virtio-auto.args | 8 +- .../qemuxml2argv-channel-virtio-autoassign.args| 20 ++ .../qemuxml2argv-channel-virtio-autoassign.xml | 50 +++ tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-channel-virtio-auto.xml | 10 +- 15 files changed, 614 insertions(+), 41 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] Add functions to track virtio-serial addresses
Store the available ports of a virtio-serial controller in a virBitmap.
The bitmaps are stored in a hash table - the controller index
formatted as a string.
Buses are not tracked, because they aren't supported by QEMU.
---
src/conf/domain_addr.c | 382 +++
src/conf/domain_addr.h | 45 ++
src/libvirt_private.syms | 8 +
3 files changed, 435 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index fb4a76f..654c95a 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -718,3 +718,385 @@ virDomainCCWAddressSetCreate(void)
virDomainCCWAddressSetFree(addrs);
return NULL;
}
+
+
+#define VIR_DOMAIN_DEFAULT_VIRTIO_SERIAL_PORTS 31
+
+
+static void
+virDomainVirtioSerialAddrHashValueFree(void *value,
+ const void *name ATTRIBUTE_UNUSED)
+{
+virBitmapPtr map = value;
+
+virBitmapFree(map);
+}
+
+/* virDomainVirtioSerialAddrSetCreate
+ *
+ * Allocates an address set for virtio serial addresses
+ */
+virDomainVirtioSerialAddrSetPtr
+virDomainVirtioSerialAddrSetCreate(void)
+{
+virDomainVirtioSerialAddrSetPtr ret = NULL;
+
+if (VIR_ALLOC(ret) 0)
+goto error;
+
+if (!(ret-used = virHashCreate(9,
virDomainVirtioSerialAddrHashValueFree)))
+goto error;
+
+return ret;
+
+ error:
+virDomainVirtioSerialAddrSetFree(ret);
+return NULL;
+}
+
+/* virDomainVirtioSerialAddrSetAddController
+ *
+ * Adds virtio serial ports of the existing controller
+ * to the address set.
+ */
+int
+virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr
addrs,
+ virDomainControllerDefPtr cont)
+{
+virBitmapPtr map = NULL;
+char *str = NULL;
+int ret = -1;
+int ports;
+
+if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
+return 0;
+
+ports = cont-opts.vioserial.ports;
+if (ports == -1)
+ports = VIR_DOMAIN_DEFAULT_VIRTIO_SERIAL_PORTS;
+
+VIR_DEBUG(Adding virtio serial controller index %u with %d
+ ports to the address set, cont-idx, ports);
+
+if (!(map = virBitmapNew(ports)))
+goto cleanup;
+
+if (virAsprintf(str, %u, cont-idx) 0)
+goto cleanup;
+
+if (virHashLookup(addrs-used, str)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(virtio serial controller with index %u
+ is already in the address set), cont-idx);
+goto cleanup;
+}
+if (virHashAddEntry(addrs-used, str, map) 0)
+goto cleanup;
+map = NULL;
+
+if (!addrs-nextInit) {
+addrs-next.controller = cont-idx;
+addrs-nextInit = true;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(str);
+virBitmapFree(map);
+return ret;
+}
+
+/* virDomainVirtioSerialAddrSetAddControllers
+ *
+ * Adds virtio serial ports of the existing controllers
+ * to the address set.
+ */
+int
+virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr
addrs,
+ virDomainDefPtr def)
+{
+size_t i;
+
+for (i = 0; i def-ncontrollers; i++) {
+if (virDomainVirtioSerialAddrSetAddController(addrs,
+ def-controllers[i]) 0)
+return -1;
+}
+
+return 0;
+}
+
+void
+virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs)
+{
+if (addrs) {
+virHashFree(addrs-used);
+VIR_FREE(addrs);
+}
+}
+
+/*
+ * Eww, this function compares two unsigned integers stored as a string
+ */
+static int
+virDomainVirtioSerialAddrCompare(const virHashKeyValuePair *a,
+ const virHashKeyValuePair *b)
+{
+const char *key_a = a-key;
+const char *key_b = b-key;
+
+size_t len_a = strlen(key_a);
+size_t len_b = strlen(key_b);
+
+/* with no padding/negative numbers allowed, the longer string
+ * contains a larger number */
+if (len_a len_b)
+return -1;
+else if (len_a len_b)
+return 1;
+else
+return strncmp(key_a, key_b, len_a);
+}
+
+static int
+virDomainVirtioSerialAddrNext(virDomainVirtioSerialAddrSetPtr addrs,
+ virDomainDeviceVirtioSerialAddress *addr,
+ bool allowZero)
+{
+virBitmapPtr cur = NULL;
+char *str = NULL;
+int ret = -1;
+virHashKeyValuePairPtr arr = NULL;
+size_t i, ncontrollers;
+size_t curidx;
+ssize_t port, start = 0;
+unsigned int controller;
+
+/* port number 0 is reserved for virtconsoles */
+if (allowZero)
+start = -1;
+
+/* What controller was the last assigned address on? */
+if (virAsprintf(str, %u, addrs-next.controller) 0)
+goto cleanup;
+
+if (!(cur = virHashLookup(addrs-used, str))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(The last used
[libvirt] [PATCH 4/5] Expand the address set when attaching a virtio-serial controller
---
src/qemu/qemu_hotplug.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 08047ce..aec541a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -435,6 +435,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
char *devstr = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
bool releaseaddr = false;
+char *contstr = NULL;
if (virDomainControllerFind(vm-def, controller-type, controller-idx) =
0) {
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -473,6 +474,13 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
goto cleanup;
}
+if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL
+virDomainVirtioSerialAddrSetAddController(priv-vioserialaddrs,
+ controller) 0)
+goto cleanup;
+if (virAsprintf(contstr, %u, controller-idx) 0)
+goto cleanup;
+
if (!(devstr = qemuBuildControllerDevStr(vm-def, controller,
priv-qemuCaps, NULL)))
goto cleanup;
}
@@ -501,10 +509,13 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
}
cleanup:
+if (ret != 0 contstr)
+virHashRemoveEntry(priv-vioserialaddrs-used, contstr);
if (ret != 0 releaseaddr)
qemuDomainReleaseDeviceAddress(vm, controller-info, NULL);
VIR_FREE(devstr);
+VIR_FREE(contstr);
return ret;
}
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] Assign an address when hotplugging a virtio-serial device
---
src/qemu/qemu_hotplug.c | 21 +++--
tests/qemuhotplugtest.c | 2 +-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index aec541a..66e052e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1532,6 +1532,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
virDomainDefPtr vmdef = vm-def;
char *devstr = NULL;
char *charAlias = NULL;
+bool need_release = false;
+bool allowZero = false;
if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -1542,6 +1544,16 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) 0)
goto cleanup;
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
+chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)
+allowZero = true;
+
+if (virDomainVirtioSerialAddrAutoAssign(priv-vioserialaddrs,
+chr-info,
+allowZero) 0)
+goto cleanup;
+need_release = true;
+
if (qemuBuildChrDeviceStr(devstr, vm-def, chr, priv-qemuCaps) 0)
goto cleanup;
@@ -1573,6 +1585,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
cleanup:
if (ret 0 virDomainObjIsActive(vm))
qemuDomainChrInsertPreAllocCleanup(vm-def, chr);
+if (ret 0 need_release)
+virDomainVirtioSerialAddrRelease(priv-vioserialaddrs, chr-info);
VIR_FREE(charAlias);
VIR_FREE(devstr);
return ret;
@@ -3969,10 +3983,13 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
goto cleanup;
rc = qemuDomainWaitForDeviceRemoval(vm);
-if (rc == 0 || rc == 1)
+if (rc == 0 || rc == 1) {
+virDomainVirtioSerialAddrRelease(priv-vioserialaddrs, tmpChr-info);
ret = qemuDomainRemoveChrDevice(driver, vm, tmpChr);
-else
+} else {
ret = 0;
+}
+
cleanup:
qemuDomainResetDeviceRemoval(vm);
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 12a7f71..ea2cf77 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -86,7 +86,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
if (event)
virQEMUCapsSet(priv-qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT);
-if (qemuDomainAssignPCIAddresses((*vm)-def, priv-qemuCaps, *vm) 0)
+if (qemuDomainAssignAddresses((*vm)-def, priv-qemuCaps, *vm) 0)
goto cleanup;
if (qemuAssignDeviceAliases((*vm)-def, priv-qemuCaps) 0)
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] Allocate virtio-serial addresses when starting a domain
Instead of always using controller 0 and incrementing port number,
respect the maximum port numbers of controllers and use all of them.
Ports for virtio consoles are quietly reserved, but not formatted
(neither in XML nor on QEMU command line).
Also rejects duplicate virtio-serial addresses.
https://bugzilla.redhat.com/show_bug.cgi?id=890606
https://bugzilla.redhat.com/show_bug.cgi?id=1076708
---
src/conf/domain_conf.c | 29 --
src/qemu/qemu_command.c| 63 ++
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_process.c| 2 +
.../qemuxml2argv-channel-virtio-auto.args | 8 +--
.../qemuxml2argv-channel-virtio-autoassign.args| 10 ++--
.../qemuxml2xmlout-channel-virtio-auto.xml | 10 ++--
8 files changed, 81 insertions(+), 43 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6da02b0..6b308cf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3349,21 +3349,6 @@
virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
chr-target.port = maxport + 1;
}
-
-if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-chr-info.addr.vioserial.port == 0) {
-int maxport = 0;
-
-for (i = 0; i cnt; i++) {
-const virDomainChrDef *thischr = arrPtr[i];
-if (thischr-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-thischr-info.addr.vioserial.controller ==
chr-info.addr.vioserial.controller
-thischr-info.addr.vioserial.bus ==
chr-info.addr.vioserial.bus
-(int)thischr-info.addr.vioserial.port maxport)
-maxport = thischr-info.addr.vioserial.port;
-}
-chr-info.addr.vioserial.port = maxport + 1;
-}
}
/* set default path for virtio-rng random backend to /dev/random */
@@ -14353,20 +14338,6 @@ virDomainDefParseXML(xmlDocPtr xml,
chr-targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO
chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
chr-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL;
-
-if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-chr-info.addr.vioserial.port == 0) {
-int maxport = 0;
-for (j = 0; j i; j++) {
-virDomainChrDefPtr thischr = def-channels[j];
-if (thischr-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-thischr-info.addr.vioserial.controller ==
chr-info.addr.vioserial.controller
-thischr-info.addr.vioserial.bus ==
chr-info.addr.vioserial.bus
-(int)thischr-info.addr.vioserial.port maxport)
-maxport = thischr-info.addr.vioserial.port;
-}
-chr-info.addr.vioserial.port = maxport + 1;
-}
}
VIR_FREE(nodes);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c112619..654fba1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1347,6 +1347,65 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
return 0;
}
+
+static int
+qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def,
+ virDomainObjPtr obj)
+{
+int ret = -1;
+size_t i;
+virDomainVirtioSerialAddrSetPtr addrs = NULL;
+qemuDomainObjPrivatePtr priv = NULL;
+
+if (!(addrs = virDomainVirtioSerialAddrSetCreate()))
+goto cleanup;
+
+if (virDomainVirtioSerialAddrSetAddControllers(addrs, def) 0)
+goto cleanup;
+
+if (virDomainDeviceInfoIterate(def, virDomainVirtioSerialAddrReserve,
+ addrs) 0)
+goto cleanup;
+
+VIR_DEBUG(Finished reserving existing ports);
+
+for (i = 0; i def-nconsoles; i++) {
+virDomainChrDefPtr chr = def-consoles[i];
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
+chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
+if (virDomainVirtioSerialAddrAssign(addrs, chr-info, true) 0)
+goto cleanup;
+}
+}
+
+for (i = 0; i def-nchannels; i++) {
+virDomainChrDefPtr chr = def-channels[i];
+if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
+chr-info.addr.vioserial.port == 0
+virDomainVirtioSerialAddrAssign(addrs, chr-info, false) 0)
+goto cleanup;
+}
+
+if (obj obj-privateData) {
+priv = obj-privateData;
+if (addrs) {
+/* if this is the live domain object, we persist the CCW
addresses*/
+virDomainVirtioSerialAddrSetFree(priv-vioserialaddrs);
+
[libvirt] [PATCH v2 1/3] utils: Implement virCommandPassFDGetFDIndex
Implement virCommandPassFDGetFDIndex to determine the index a given
file descriptor will have when passed to the child process.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
src/libvirt_private.syms | 1 +
src/util/vircommand.c| 24
src/util/vircommand.h| 3 +++
3 files changed, 28 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6028002..96cfbf2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1214,6 +1214,7 @@ virCommandNewArgList;
virCommandNewArgs;
virCommandNonblockingFDs;
virCommandPassFD;
+virCommandPassFDGetFDIndex;
virCommandPassListenFDs;
virCommandRawStatus;
virCommandRequireHandshake;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 6527d85..648f5ed 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1019,6 +1019,30 @@ virCommandPassListenFDs(virCommandPtr cmd)
cmd-flags |= VIR_EXEC_LISTEN_FDS;
}
+/*
+ * virCommandPassFDGetFDIndex:
+ * @cmd: pointer to virCommand
+ * @fd: FD to get index of
+ *
+ * Determine the index of the FD in the transfer set.
+ *
+ * Returns index = 0 if @set contains @fd,
+ * -1 otherwise.
+ */
+int
+virCommandPassFDGetFDIndex(virCommandPtr cmd, int fd)
+{
+size_t i = 0;
+
+while (i cmd-npassfd) {
+if (cmd-passfd[i].fd == fd)
+return i;
+i++;
+}
+
+return -1;
+}
+
/**
* virCommandSetPidFile:
* @cmd: the command to modify
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index bf65de4..198da2f 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -62,6 +62,9 @@ void virCommandPassFD(virCommandPtr cmd,
void virCommandPassListenFDs(virCommandPtr cmd);
+int virCommandPassFDGetFDIndex(virCommandPtr cmd,
+ int fd);
+
void virCommandSetPidFile(virCommandPtr cmd,
const char *pidfile) ATTRIBUTE_NONNULL(2);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 25/31] test_driver: Use virNetworkObjEndAPI
On Thu, Feb 26, 2015 at 15:17:34 +0100, Michal Privoznik wrote: So far, this is pure code replacement. But once we introduce reference counting to virNetworkObj this will be more handy as there'll be only one function to change: virNetworkObjEndAPI(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/test/test_driver.c | 42 ++ 1 file changed, 14 insertions(+), 28 deletions(-) ACK signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] qemu: Move TPM command line build code into own function
Move the TPM command line build code into its own function.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
src/qemu/qemu_command.c | 37 +
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c112619..d6bc294 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8191,6 +8191,30 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
return ret;
}
+static int
+qemuBuildTPMCommandLine(virDomainDefPtr def,
+virCommandPtr cmd,
+virQEMUCapsPtr qemuCaps,
+const char *emulator)
+{
+char *optstr;
+
+if (!(optstr = qemuBuildTPMBackendStr(def, qemuCaps, emulator)))
+return -1;
+
+virCommandAddArgList(cmd, -tpmdev, optstr, NULL);
+VIR_FREE(optstr);
+
+if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator)))
+return -1;
+
+virCommandAddArgList(cmd, -device, optstr, NULL);
+VIR_FREE(optstr);
+
+return 0;
+}
+
+
qemuBuildCommandLineCallbacks buildCommandLineCallbacks = {
.qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName,
};
@@ -9600,19 +9624,8 @@ qemuBuildCommandLine(virConnectPtr conn,
}
if (def-tpm) {
-char *optstr;
-
-if (!(optstr = qemuBuildTPMBackendStr(def, qemuCaps, emulator)))
+if (qemuBuildTPMCommandLine(def, cmd, qemuCaps, emulator) 0)
goto error;
-
-virCommandAddArgList(cmd, -tpmdev, optstr, NULL);
-VIR_FREE(optstr);
-
-if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator)))
-goto error;
-
-virCommandAddArgList(cmd, -device, optstr, NULL);
-VIR_FREE(optstr);
}
for (i = 0; i def-ninputs; i++) {
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] qemu: Pass file descriptor when using TPM passthrough
Pass the TPM file descriptor to QEMU via command line.
Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional
parameters -add-fd set=10,fd=20.
This addresses the use case when QEMU is started with non-root privileges
and QEMU cannot open /dev/tpm0 for example.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
src/qemu/qemu_command.c | 121 ++--
1 file changed, 117 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6bc294..4c6b76d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -161,6 +161,58 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
interleave);
/**
+ * qemuVirCommandGetFDSet:
+ * @cmd: the command to modify
+ * @fd: fd to reassign to the child
+ *
+ * Get the parameters for the QEMU -add-fd command line option
+ * for the given file descriptor. The file descriptor must previously
+ * have been 'transferred' in a virCommandPassFD() call.
+ * This function for example returns set=10,fd=20.
+ */
+static char *
+qemuVirCommandGetFDSet(virCommandPtr cmd, int fd)
+{
+char *result = NULL;
+int idx = virCommandPassFDGetFDIndex(cmd, fd);
+
+if (idx = 0) {
+ignore_value(virAsprintf(result, set=%d,fd=%d, idx, fd));
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(file descriptor %d has not been transferred), fd);
+}
+
+return result;
+}
+
+/**
+ * qemuVirCommandGetDevSet:
+ * @cmd: the command to modify
+ * @fd: fd to reassign to the child
+ *
+ * Get the parameters for the QEMU path= parameter where a file
+ * descriptor is accessed via a file descriptor set, for example
+ * /dev/fdset/10. The file descriptor must previously have been
+ * 'transferred' in a virCommandPassFD() call.
+ */
+static char *
+qemuVirCommandGetDevSet(virCommandPtr cmd, int fd)
+{
+char *result = NULL;
+int idx = virCommandPassFDGetFDIndex(cmd, fd);
+
+if (idx = 0) {
+ignore_value(virAsprintf(result, /dev/fdset/%d, idx));
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(file descriptor %d has not been transferred), fd);
+}
+return result;
+}
+
+
+/**
* qemuPhysIfaceConnect:
* @def: the definition of the VM (needed by 802.1Qbh and audit)
* @driver: pointer to the driver instance
@@ -6321,15 +6373,20 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
static char *qemuBuildTPMBackendStr(const virDomainDef *def,
+virCommandPtr cmd,
virQEMUCapsPtr qemuCaps,
-const char *emulator)
+const char *emulator,
+int *tpmfd, int *cancelfd)
{
const virDomainTPMDef *tpm = def-tpm;
virBuffer buf = VIR_BUFFER_INITIALIZER;
const char *type = virDomainTPMBackendTypeToString(tpm-type);
-char *cancel_path;
+char *cancel_path = NULL, *devset = NULL;
const char *tpmdev;
+*tpmfd = -1;
+*cancelfd = -1;
+
virBufferAsprintf(buf, %s,id=tpm-%s, type, tpm-info.alias);
switch (tpm-type) {
@@ -6341,11 +6398,42 @@ static char *qemuBuildTPMBackendStr(const virDomainDef
*def,
if (!(cancel_path = virTPMCreateCancelPath(tpmdev)))
goto error;
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) {
+*tpmfd = open(tpmdev, O_RDWR);
+if (*tpmfd 0) {
+virReportSystemError(errno, _(Could not open TPM device %s),
+ tpmdev);
+goto error;
+}
+
+virCommandPassFD(cmd, *tpmfd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+devset = qemuVirCommandGetDevSet(cmd, *tpmfd);
+if (devset == NULL)
+goto error;
+
+*cancelfd = open(cancel_path, O_WRONLY);
+if (*cancelfd 0) {
+virReportSystemError(errno,
+ _(Could not open TPM device's cancel
+ path %s), cancel_path);
+goto error;
+}
+VIR_FREE(cancel_path);
+
+virCommandPassFD(cmd, *cancelfd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+cancel_path = qemuVirCommandGetDevSet(cmd, *cancelfd);
+if (cancel_path == NULL)
+goto error;
+}
virBufferAddLit(buf, ,path=);
-virBufferEscape(buf, ',', ,, %s, tpmdev);
+virBufferEscape(buf, ',', ,, %s, devset ? devset : tpmdev);
virBufferAddLit(buf, ,cancel-path=);
virBufferEscape(buf, ',', ,, %s, cancel_path);
+
+VIR_FREE(devset);
VIR_FREE(cancel_path);
break;
@@ -6365,6 +6453,9 @@ static char *qemuBuildTPMBackendStr(const virDomainDef
*def,
Re: [libvirt] [PATCH v1 27/31] virNetworkObjListPtr: Make APIs self-locking
On Thu, Feb 26, 2015 at 15:17:36 +0100, Michal Privoznik wrote:
Every API that touches internal structure of the object must lock
the object first. Not every API that has the object as an
argument needs to do that though. Some APIs just pass the object
to lower layers, which, however, must lock the object then. Look
Too, many, commas.
at the code, you'll get my meaning soon.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 39 +--
1 file changed, 33 insertions(+), 6 deletions(-)
@@ -631,6 +644,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
size_t i;
virObjectUnlock(net);
+virObjectLock(nets);
Hmm, okay. My comment to the previous patch is invalid as this would
actually introduce a lock ordering problem.
for (i = 0; i nets-count; i++) {
virObjectLock(nets-objs[i]);
if (nets-objs[i] == net) {
@@ -642,6 +656,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
}
virObjectUnlock(nets-objs[i]);
}
+virObjectUnlock(nets);
}
/* return ips[index], or NULL if there aren't enough ips */
ACK
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
