Re: [libvirt] [PATCH] Give correct error message when configure a wrong URI aliase.
On Tue, Nov 22, 2011 at 11:27:04AM +0800, ta...@linux.vnet.ibm.com wrote: > From: Eli Qiao > > Signed-off-by: Eli Qiao > > When configure the URI aliase like this in 'libvirt.conf': > > uri_aliases = [ > "jj#j=qemu+ssh://root@127.0.0.1/system", > "sleet=qemu+ssh://r...@sleet.cloud.example.com/system", > ] > virsh -c jj#j No, we don't want # to appear in aliases as this will lead to confusion > It will show this error message: > 'no connection driver available for No connection for URI jj#j' > Actually ，we expect this message below： > Malformed 'uri_aliases' config entry 'jj#j=qemu+ssh://root@127.0.0.1/system', > aliases may only container 'a-Z, 0-9, _, -' > > Give this patch to fix this error. > --- > src/libvirt.c |5 - > 1 files changed, 0 insertions(+), 5 deletions(-) > > diff --git a/src/libvirt.c b/src/libvirt.c > index 1518ed2..17e073e 100644 > --- a/src/libvirt.c > +++ b/src/libvirt.c > @@ -1054,11 +1054,6 @@ virConnectOpenResolveURIAlias(const char *alias, char > **uri) > > *uri = NULL; > > -/* Short circuit to avoid doing URI alias resolution > - * when it clearly isn't an valid alias */ > -if (strspn(alias, URI_ALIAS_CHARS) != strlen(alias)) > -return 0; > - > if (!(config = virConnectConfigFile())) > goto cleanup; I disagree, I don't see any need to have complex aliases values. The principle is precisely to keep them simple. Use "jj" instead of "jj#j" for example. NACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Give correct error message when configure a wrong URI aliase.
On 2011年11月22日 16:07, Daniel Veillard wrote: On Tue, Nov 22, 2011 at 11:27:04AM +0800, ta...@linux.vnet.ibm.com wrote: From: Eli Qiao Signed-off-by: Eli Qiao When configure the URI aliase like this in 'libvirt.conf': uri_aliases = [ "jj#j=qemu+ssh://root@127.0.0.1/system", "sleet=qemu+ssh://r...@sleet.cloud.example.com/system", ] virsh -c jj#j No, we don't want # to appear in aliases as this will lead to confusion I think Eli's purpose is to see the more sensiable message by removing the early checking. But I don't see the more sensiable message with applying the patch. It will show this error message: 'no connection driver available for No connection for URI jj#j' Actually ，we expect this message below： Malformed 'uri_aliases' config entry 'jj#j=qemu+ssh://root@127.0.0.1/system', aliases may only container 'a-Z, 0-9, _, -' Give this patch to fix this error. --- src/libvirt.c |5 - 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 1518ed2..17e073e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1054,11 +1054,6 @@ virConnectOpenResolveURIAlias(const char *alias, char **uri) *uri = NULL; -/* Short circuit to avoid doing URI alias resolution - * when it clearly isn't an valid alias */ -if (strspn(alias, URI_ALIAS_CHARS) != strlen(alias)) -return 0; - if (!(config = virConnectConfigFile())) goto cleanup; I disagree, I don't see any need to have complex aliases values. The principle is precisely to keep them simple. Use "jj" instead of "jj#j" for example. Disagree too. The simple early checking ensure it doesn't flow into the loop, which is just waste. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] problem with nwfilter and ip6tables
That actually works, thanks a lot! Op 21-11-2011 17:07, Stefan Berger schreef: On 11/21/2011 08:46 AM, Reinier Schoof wrote: Going back to the original email: Hi, I'm investigating using the nwfilter-functionality of libvirt to give my clients the possibility to block ports of their VPSes. The same mechanism allows me to restrict the outgoing traffic a VPS is generating. In the end, I want to restrict MAC, IPv4 and IPv6 traffic, while the client can also restrict traffic to UDP and TCP. All goes well, until I want to restrict the UDP/TCP traffic to certain IPv6 addresses. Where iptables shows the IPv4-restriction I've put up, ip6tables doesn't show anything. In the logs, I only see some ip6tables -D, -X and -F commands failing, which is expected when libvirt tries to delete/flush rules that were never there. I've built my nwfilter containing the following IPv6-rules, which I for instance reference once for all the TCP-ports which should be open. Replace with d7ca42fe-a2f5-6491-cdee-10d8a0956772 Replace with ff97e825-712d-6b1a-c5d1-46fe635f9dd6 This probably should either be direction='in' or you may want to replace srcipaddr and srcipmask with dstipaddr and dstipmask. Replace with 6e738070-9505-730d-14e6-ee01a6eb5885 You may want to add state='NEW' to the rule as well. Replace with 4377aca7-18fb-b373-4462-4ee2ba3db7cd You have to change the chain to 'root' and the protocol in the rules has to be tcp-ipv6, all-ipv6 etc. for ipv6 traffic. The reason is that most of these rules could be applied to either iptables or ip6tables and the network filtering system needs some more 'hints' whether it is indeed an ipv6 rule so it create ip6tables commands versus iptables commands. I hope this helps. 'ip6tables -L -n' here now shows: Chain FI-vnet0 (1 references) target prot opt source destination RETURN all ::/0 ::/0 state RELATED,ESTABLISHED RETURN all ::/0 ::/64 state ESTABLISHED ctdir ORIGINAL DROP all ::/0 ::/0 Chain FO-vnet0 (1 references) target prot opt source destination ACCEPT all ::/0 ::/0 state ESTABLISHED ACCEPT tcp ::/0 ::/0 tcp dpt:90 state NEW ACCEPT udp ::/0 ::/0 udp dpt:90 state NEW ACCEPT all ::/64 ::/0 state NEW,ESTABLISHED ctdir REPLY DROP all ::/0 ::/0 Chain HI-vnet0 (1 references) target prot opt source destination RETURN all ::/0 ::/0 state RELATED,ESTABLISHED RETURN all ::/0 ::/64 state ESTABLISHED ctdir ORIGINAL DROP all ::/0 ::/0 Stefan I use a similar approach for my IPv4 firewall, and it works perfectly. When I use these IPv6 rules, all IPv6 traffic is apparently dropped, but it's hard to debug when the result of this config is abscent in ip6tables. I'm using these version of software on debian 6.0 squeeze: virsh # version Compiled against library: libvir 0.9.2 Using library: libvir 0.9.2 Using API: QEMU 0.9.2 Running hypervisor: QEMU 0.15.0 Does anyone have any clues? Thanks in advance! Regards, Reinier Schoof -- TransIP BV | https://www.transip.nl/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Update clean part for cases
On 11/21/2011 03:23 PM, Wayne Sun wrote:
* add clean function for missing ones
* clean the system configuration files which been touched
---
repos/domain/balloon_memory.py |2 ++
repos/domain/cpu_topology.py |5 +
repos/domain/ownership_test.py | 10 ++
repos/libvirtd/qemu_hang.py | 12 ++--
repos/remoteAccess/unix_perm_sasl.py | 10 ++
repos/sVirt/domain_nfs_start.py | 10 ++
repos/snapshot/delete.py | 18 +++---
repos/snapshot/file_flag.py | 28 +---
repos/snapshot/flag_check.py |4 +++-
repos/snapshot/internal_create.py|4 +++-
repos/snapshot/revert.py |3 +++
repos/snapshot/snapshot_list.py | 16 +++-
12 files changed, 67 insertions(+), 55 deletions(-)
diff --git a/repos/domain/balloon_memory.py b/repos/domain/balloon_memory.py
index 30f5edb..0a40591 100644
--- a/repos/domain/balloon_memory.py
+++ b/repos/domain/balloon_memory.py
@@ -299,6 +299,8 @@ def balloon_memory(params):
logger.info("the actual size of memory is \
rounded to the value %s we expected" % maxmem)
+util.clean_ssh()
+
if count:
return return_close(conn, logger, 1)
else:
diff --git a/repos/domain/cpu_topology.py b/repos/domain/cpu_topology.py
index 7202559..adf238c 100644
--- a/repos/domain/cpu_topology.py
+++ b/repos/domain/cpu_topology.py
@@ -246,3 +246,8 @@ def cpu_topology(params):
conn.close()
return 0
+
+def cpu_topology_clean(params):
+"""clean testing enviorment"""
+return 0
+
diff --git a/repos/domain/ownership_test.py b/repos/domain/ownership_test.py
index 74620f4..1eacbcf 100644
--- a/repos/domain/ownership_test.py
+++ b/repos/domain/ownership_test.py
@@ -307,6 +307,9 @@ def ownership_test_clean(params):
for i in range(len(out)):
logger.error(out[i])
+clean_nfs_conf = "sed -i '$d' /etc/exports"
+util.exec_cmd(clean_nfs_conf, shell=True)
+
filepath = TEMP_FILE
elif use_nfs == 'disable':
filepath = SAVE_FILE
@@ -314,3 +317,10 @@ def ownership_test_clean(params):
if os.path.exists(filepath):
os.remove(filepath)
+clean_qemu_conf = "sed -i '$d' %s" % QEMU_CONF
+util.exec_cmd(clean_qemu_conf, shell=True)
+
+cmd = "service libvirtd restart"
+util.exec_cmd(cmd, shell=True)
+
+return 0
diff --git a/repos/libvirtd/qemu_hang.py b/repos/libvirtd/qemu_hang.py
index c97e0b7..8b1ffda 100644
--- a/repos/libvirtd/qemu_hang.py
+++ b/repos/libvirtd/qemu_hang.py
@@ -135,8 +135,16 @@ def qemu_hang(params):
return 0
-def qemu_hang_cleanup(params):
+def qemu_hang_clean(params):
""" clean testing environment """
-pass
+logger = params['logger']
+guestname = params['guestname']
+util = utils.Utils()
+ret = get_domain_pid(util, logger, guestname)
+cmd = "kill -CONT %s" % ret[1]
+ret = util.exec_cmd(cmd, shell=True)
+if ret[0]:
+logger.error("failed to resume qemu process of %s" % guestname)
+return 0
diff --git a/repos/remoteAccess/unix_perm_sasl.py
b/repos/remoteAccess/unix_perm_sasl.py
index 54fa108..5db758c 100644
--- a/repos/remoteAccess/unix_perm_sasl.py
+++ b/repos/remoteAccess/unix_perm_sasl.py
@@ -22,6 +22,7 @@ import sys
import commands
from pwd import getpwnam
+from utils.Python import utils
def append_path(path):
"""Append root path of package"""
@@ -214,6 +215,7 @@ def unix_perm_sasl(params):
def unix_perm_sasl_clean(params):
"""clean testing environment"""
logger = params['logger']
+util = utils.Utils()
auth_unix_ro = params['auth_unix_ro']
auth_unix_rw = params['auth_unix_rw']
@@ -241,3 +243,11 @@ def unix_perm_sasl_clean(params):
if status:
logger.error("failed to delete sasl user %s" % TESTING_USER)
+clean_libvirtd_conf = "sed -i -e :a -e '$d;N;2,3ba' -e 'P;D' %s" % \
+ LIBVIRTD_CONF
+util.exec_cmd(clean_libvirtd_conf, shell=True)
+
+cmd = "service libvirtd restart"
+util.exec_cmd(cmd, shell=True)
+
+return 0
diff --git a/repos/sVirt/domain_nfs_start.py b/repos/sVirt/domain_nfs_start.py
index 2479366..edaf2f2 100644
--- a/repos/sVirt/domain_nfs_start.py
+++ b/repos/sVirt/domain_nfs_start.py
@@ -483,3 +483,13 @@ def domain_nfs_start_clean(params):
conn.close()
+clean_nfs_conf = "sed -i '$d' /etc/exports"
+util.exec_cmd(clean_nfs_conf, shell=True)
+
+clean_qemu_conf = "sed -i '$d' %s" % QEMU_CONF
+util.exec_cmd(clean_qemu_conf, shell=True)
+
+cmd = "service libvirtd restart"
+util.exec_cmd(cmd, shell=True)
+
+return 0
diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py
index 8a7eef6..98f82a5 100644
--- a/repos/snapshot/delete.py
+++ b/repos/snapshot/delete.py
@@ -124,18 +124,6 @@ def delete(params)
Re: [libvirt] [libvirt-glib] Add API to redefine an existing domain
On Mon, Nov 21, 2011 at 08:27:17PM +0100, Marc-André Lureau wrote:
> Hi
>
> On Mon, Nov 21, 2011 at 6:53 PM, Zeeshan Ali (Khattak)
> wrote:
> > + g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
>
> This is wrong, it should be error != NULL && *error == NULL.
>
> > + if (virDomainDefineXML(conn, xml) == NULL) {
> > + if (error != NULL)
> > + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> > + 0,
> > + "Failed to set "
> > + "domain configuration");
> > + return FALSE;
> > + }
>
> Can you please verify that the return value is safe to ignore?
>
> I would really prefer we add a runtime check that verifiy the handle
> is the same as the one currently associated with the domain.
The actual pointer returned will *not* be the same as the same
one you currently have, but assuming the XML has matching UUID
and name, it will point to the same domain.
So I guess what you want todo is verify the UUID and name in
the XML, before defining it.
Also, you need to call virDomainFree on the return value of
virDomainDefineXML to avoid memleak.
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] Give correct error message when configure a wrong URI aliase.
On Tue, Nov 22, 2011 at 04:07:21PM +0800, Daniel Veillard wrote: > On Tue, Nov 22, 2011 at 11:27:04AM +0800, ta...@linux.vnet.ibm.com wrote: > > From: Eli Qiao > > > > Signed-off-by: Eli Qiao > > > > When configure the URI aliase like this in 'libvirt.conf': > > > > uri_aliases = [ > > "jj#j=qemu+ssh://root@127.0.0.1/system", > > "sleet=qemu+ssh://r...@sleet.cloud.example.com/system", > > ] > > virsh -c jj#j > > No, we don't want # to appear in aliases as this will lead to > confusion > > > It will show this error message: > > 'no connection driver available for No connection for URI jj#j' > > Actually ，we expect this message below： > > Malformed 'uri_aliases' config entry > > 'jj#j=qemu+ssh://root@127.0.0.1/system', aliases may only container 'a-Z, > > 0-9, _, -' > > > > Give this patch to fix this error. > > --- > > src/libvirt.c |5 - > > 1 files changed, 0 insertions(+), 5 deletions(-) > > > > diff --git a/src/libvirt.c b/src/libvirt.c > > index 1518ed2..17e073e 100644 > > --- a/src/libvirt.c > > +++ b/src/libvirt.c > > @@ -1054,11 +1054,6 @@ virConnectOpenResolveURIAlias(const char *alias, > > char **uri) > > > > *uri = NULL; > > > > -/* Short circuit to avoid doing URI alias resolution > > - * when it clearly isn't an valid alias */ > > -if (strspn(alias, URI_ALIAS_CHARS) != strlen(alias)) > > -return 0; > > - > > if (!(config = virConnectConfigFile())) > > goto cleanup; > > I disagree, I don't see any need to have complex aliases values. > The principle is precisely to keep them simple. Use "jj" instead of "jj#j" > for example. > > NACK Actually, the patch doesn't change what is allowed - it still forbids 'jj#j' later on in the code. This only changes error reporting to be more clear, so I ACK this. 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] Give correct error message when configure a wrong URI aliase.
On Tue, Nov 22, 2011 at 11:27:04AM +0800, ta...@linux.vnet.ibm.com wrote: > From: Eli Qiao > > Signed-off-by: Eli Qiao > > When configure the URI aliase like this in 'libvirt.conf': > > uri_aliases = [ > "jj#j=qemu+ssh://root@127.0.0.1/system", > "sleet=qemu+ssh://r...@sleet.cloud.example.com/system", > ] > virsh -c jj#j > > It will show this error message: > 'no connection driver available for No connection for URI jj#j' > Actually ，we expect this message below： > Malformed 'uri_aliases' config entry 'jj#j=qemu+ssh://root@127.0.0.1/system', > aliases may only container 'a-Z, 0-9, _, -' > > Give this patch to fix this error. > --- > src/libvirt.c |5 - > 1 files changed, 0 insertions(+), 5 deletions(-) > > diff --git a/src/libvirt.c b/src/libvirt.c > index 1518ed2..17e073e 100644 > --- a/src/libvirt.c > +++ b/src/libvirt.c > @@ -1054,11 +1054,6 @@ virConnectOpenResolveURIAlias(const char *alias, char > **uri) > > *uri = NULL; > > -/* Short circuit to avoid doing URI alias resolution > - * when it clearly isn't an valid alias */ > -if (strspn(alias, URI_ALIAS_CHARS) != strlen(alias)) > -return 0; > - > if (!(config = virConnectConfigFile())) > goto cleanup; > ACK, this is OK, since we still validate the URI_ALIAS_CHARS later on Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add strings.h include to capabilities.h for ffs() function prototype
From: "Daniel P. Berrange" Pushed the following to fix the build on mingw32 On Mingw32 the ffs() function was not declared due to missing header include * src/conf/capabilities.c: The ffs() function lives in strings.h --- src/conf/capabilities.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 87b60b0..ac132f9 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -23,6 +23,8 @@ #include +#include + #include "capabilities.h" #include "buf.h" #include "memory.h" -- 1.7.6.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] API to invoke S3/S4 on a host and also resume from within libvirt
On Mon, Nov 21, 2011 at 05:26:55PM -0700, Eric Blake wrote: > On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote: > > (This patch is positioned to go in after the patch that exports the host > > power management capabilities as XML, posted in [4]) > > I'm now reviewing that patch along with this series; if we need another > revision, it may be easiest to merge the three patches into a single > series and title it v6, so that they are together in one thread. > > > > > This patch adds a new API to put a host to a suspended state (either > > Suspend-to-RAM or Suspend-to-Disk) and setup a timed resume to get the > > host back online, from within libvirt. > > This uses the RTC wakeup mechanism to set up a timer alarm before > > suspending the host, so that in-band resume is facilitated by the firing > > of the RTC alarm, which wakes up the host. > > I'm not as familiar with S3/S4 as I would like to be - am I correct that > RTC wakeup works for S3 (where the host is maintaining minimal power and > can thus react to interrupts), but not S4 (where the host has flushed > completely to disk and powers off, but resumes from the state on disk on > next power on)? Or am I misunderstanding these two power-saving states? Theoretically I think you're right, but in current qemu S3 does a hardware reset without reseting the memory, so kind of a sleep + immediate wakeup. S4 is emulated correctly - qemu doesn't have to do anything special. > > > > > The decision to use the RTC Wakeup mechanism to resume the host from > > sleep was taken in [1]. An initial API was discussed in [2]. > > Some design ideas for the asynchronous mechanism implementation was > > discussed in [3]. > > > > v3: > >* Rebased to libvirt 0.9.7 > >* Added a check to see if alarmTime (suspend duration) is within an > > acceptable range. > > > > v2: > >* Added an init function which finds out if S3/S4 is supported by the > > host, > > upon the first request to suspend/hibernate. > >* Added synchronization/locking to ensure that only one suspend operation > > is active at a time. > > > > v1: http://www.redhat.com/archives/libvir-list/2011-September/msg00830.html > > v2: http://comments.gmane.org/gmane.comp.emulators.libvirt/46729 > > > > References: > > [1]. http://www.redhat.com/archives/libvir-list/2011-August/msg00327.html > > [2]. http://www.redhat.com/archives/libvir-list/2011-August/msg00248.html > > [3]. http://www.redhat.com/archives/libvir-list/2011-September/msg00438.html > > [4]. http://www.redhat.com/archives/libvir-list/2011-November/msg00378.html > > > > -- > Eric Blake ebl...@redhat.com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4] virsh: Increase device-detach intelligence
From: Michal Prívozník
Up to now users have to give a full XML description on input when
device-detaching. If they omitted something it lead to unclear
error messages (like generated MAC wasn't found, etc.).
With this patch users can specify only those information which
specify one device sufficiently precise. Remaining information is
completed from domain.
---
diff to v3:
- Eric's review:
https://www.redhat.com/archives/libvir-list/2011-September/msg00472.html
tools/virsh.c | 284 +---
1 files changed, 268 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 89fb4e7..a798561 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -60,6 +60,7 @@
#include "command.h"
#include "virkeycode.h"
#include "virnetdevbandwidth.h"
+#include "util/bitmap.h"
static char *progname;
@@ -11502,6 +11503,243 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
return true;
}
+/**
+ * Check if n1 is superset of n2, meaning n1 contains all elements and
+ * attributes as n2 at least. Including children.
+ * @n1 first node
+ * @n2 second node
+ * return 1 in case n1 covers n2, 0 otherwise.
+ */
+static bool
+vshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2) {
+xmlNodePtr child1, child2;
+xmlAttrPtr attr;
+char *prop1, *prop2;
+bool found;
+bool visited;
+bool ret = false;
+unsigned long n1_child_size, n1_iter;
+virBitmapPtr bitmap;
+
+if (!n1 && !n2)
+return true;
+
+if (!n1 || !n2)
+return false;
+
+if (!xmlStrEqual(n1->name, n2->name))
+return false;
+
+/* Iterate over n2 attributes and check if n1 contains them*/
+attr = n2->properties;
+while (attr) {
+if (attr->type == XML_ATTRIBUTE_NODE) {
+prop1 = virXMLPropString(n1, (const char *) attr->name);
+prop2 = virXMLPropString(n2, (const char *) attr->name);
+if (STRNEQ_NULLABLE(prop1, prop2)) {
+xmlFree(prop1);
+xmlFree(prop2);
+return false;
+}
+xmlFree(prop1);
+xmlFree(prop2);
+}
+attr = attr->next;
+}
+
+n1_child_size = xmlChildElementCount(n1);
+if (!n1_child_size) {
+return !xmlChildElementCount(n2);
+}
+
+if (!(bitmap = virBitmapAlloc(n1_child_size))) {
+virReportOOMError();
+return false;
+}
+
+child2 = n2->children;
+while (child2) {
+if (child2->type != XML_ELEMENT_NODE) {
+child2 = child2->next;
+continue;
+}
+
+child1 = n1->children;
+n1_iter = 0;
+found = false;
+while (child1) {
+if (child1->type != XML_ELEMENT_NODE) {
+child1 = child1->next;
+continue;
+}
+
+if (virBitmapGetBit(bitmap, n1_iter, &visited) < 0) {
+vshError(NULL, "%s", _("Bad child elements counting."));
+goto cleanup;
+}
+
+if (visited) {
+child1 = child1->next;
+n1_iter++;
+continue;
+}
+
+if (xmlStrEqual(child1->name, child2->name)) {
+found = true;
+if (virBitmapSetBit(bitmap, n1_iter) < 0) {
+vshError(NULL, "%s", _("Bad child elements counting."));
+goto cleanup;
+}
+
+if (!vshNodeIsSuperset(child1, child2))
+goto cleanup;
+
+break;
+}
+
+child1 = child1->next;
+n1_iter++;
+}
+
+if (!found)
+goto cleanup;
+
+child2 = child2->next;
+}
+
+ret = true;
+
+cleanup:
+virBitmapFree(bitmap);
+return ret;
+}
+
+/**
+ * vshCompleteXMLFromDomain:
+ * @ctl vshControl for error messages printing
+ * @dom domain
+ * @oldXML device XML before
+ * @newXML and after completion
+ *
+ * For given domain and (probably incomplete) device XML specification try to
+ * find such device in domain and complete missing parts. This is however
+ * possible only when given device XML is sufficiently precise so it addresses
+ * only one device.
+ *
+ * Returns -2 when no such device exists in domain, -3 when given XML selects
many
+ * (is too ambiguous), 0 in case of success. Otherwise returns -1.
@newXML
+ * is touched only in case of success.
+ */
+static int
+vshCompleteXMLFromDomain(vshControl *ctl, virDomainPtr dom, char *oldXML,
+ char **newXML) {
+int funcRet = -1;
+char *domXML = NULL;
+xmlDocPtr domDoc = NULL, devDoc = NULL;
+xmlNodePtr node = NULL;
+xmlXPathContextPtr domCtxt = NULL, devCtxt = NULL;
+xmlNodePtr *devices = NULL;
+xmlSaveCtxtPtr sctxt = NULL;
+int devices_size;
+char *xpath;
+xmlBufferPtr buf = NULL;
+int i = 0;
+int indx = -1;
+
+if (!(domXML = vi
[libvirt] New feature for libvirt network filter
Hi, I wanted to suggest a new feature in libvirt network filter. Currently libvirt allow only one ip address to be provided in the guest configuration xml file. We should allow multiple ip addresses to be provided statically in the guest xml for an interface, so that packets matching either of these multiple ip addresses should be allowed to be routed out from the guest machine. Reagrds Amit tTewari NHST DISCLAIMER: --- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . - libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] API to invoke S3/S4 on a host and also resume from within libvirt
On 11/22/2011 05:56 AM, Eric Blake wrote: > On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote: >> (This patch is positioned to go in after the patch that exports the host >> power management capabilities as XML, posted in [4]) > > I'm now reviewing that patch along with this series; if we need another > revision, it may be easiest to merge the three patches into a single > series and title it v6, so that they are together in one thread. > >> >> This patch adds a new API to put a host to a suspended state (either >> Suspend-to-RAM or Suspend-to-Disk) and setup a timed resume to get the >> host back online, from within libvirt. >> This uses the RTC wakeup mechanism to set up a timer alarm before >> suspending the host, so that in-band resume is facilitated by the firing >> of the RTC alarm, which wakes up the host. > > I'm not as familiar with S3/S4 as I would like to be - am I correct that > RTC wakeup works for S3 (where the host is maintaining minimal power and > can thus react to interrupts), but not S4 (where the host has flushed > completely to disk and powers off, but resumes from the state on disk on > next power on)? Or am I misunderstanding these two power-saving states? > Actually, RTC wakeup works for both S3 and S4. Hence this patch can use that mechanism effectively for both the system-wide sleep states. Thanks, Srivatsa S. Bhat IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] API to invoke S3/S4 on a host and also resume from within libvirt
On 11/22/2011 03:48 PM, Alon Levy wrote: > On Mon, Nov 21, 2011 at 05:26:55PM -0700, Eric Blake wrote: >> On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote: >>> (This patch is positioned to go in after the patch that exports the host >>> power management capabilities as XML, posted in [4]) >> >> I'm now reviewing that patch along with this series; if we need another >> revision, it may be easiest to merge the three patches into a single >> series and title it v6, so that they are together in one thread. >> >>> >>> This patch adds a new API to put a host to a suspended state (either >>> Suspend-to-RAM or Suspend-to-Disk) and setup a timed resume to get the >>> host back online, from within libvirt. >>> This uses the RTC wakeup mechanism to set up a timer alarm before >>> suspending the host, so that in-band resume is facilitated by the firing >>> of the RTC alarm, which wakes up the host. >> >> I'm not as familiar with S3/S4 as I would like to be - am I correct that >> RTC wakeup works for S3 (where the host is maintaining minimal power and >> can thus react to interrupts), but not S4 (where the host has flushed >> completely to disk and powers off, but resumes from the state on disk on >> next power on)? Or am I misunderstanding these two power-saving states? > > Theoretically I think you're right, but in current qemu S3 does a > hardware reset without reseting the memory, so kind of a sleep + > immediate wakeup. S4 is emulated correctly - qemu doesn't have to do > anything special. > Hi Alon, Actually this patchset talks about S3/S4 on the _host_, not the guest. So, in the host, RTC wakeup works for both S3 and S4. Thanks, Srivatsa S. Bhat IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-glib 0/5] Misc fixes & enhancements
NB, this is an ABI changing series, to fix the bogus use of guint64 for flags parameters -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-glib 2/5] Change all flags from guint64 to guint to match libvirt type
From: "Daniel P. Berrange"
---
libvirt-gobject/libvirt-gobject-connection.c |6 ++--
libvirt-gobject/libvirt-gobject-connection.h |4 +-
libvirt-gobject/libvirt-gobject-domain-snapshot.c |2 +-
libvirt-gobject/libvirt-gobject-domain-snapshot.h |2 +-
libvirt-gobject/libvirt-gobject-domain.c | 37 ++--
libvirt-gobject/libvirt-gobject-domain.h | 16
libvirt-gobject/libvirt-gobject-interface.c |2 +-
libvirt-gobject/libvirt-gobject-interface.h |2 +-
libvirt-gobject/libvirt-gobject-network-filter.c |2 +-
libvirt-gobject/libvirt-gobject-network-filter.h |2 +-
libvirt-gobject/libvirt-gobject-network.c |2 +-
libvirt-gobject/libvirt-gobject-network.h |2 +-
libvirt-gobject/libvirt-gobject-node-device.c |2 +-
libvirt-gobject/libvirt-gobject-node-device.h |2 +-
libvirt-gobject/libvirt-gobject-secret.c |2 +-
libvirt-gobject/libvirt-gobject-secret.h |2 +-
libvirt-gobject/libvirt-gobject-storage-pool.c| 12 +++---
libvirt-gobject/libvirt-gobject-storage-pool.h| 10 +++---
libvirt-gobject/libvirt-gobject-storage-vol.c |2 +-
libvirt-gobject/libvirt-gobject-storage-vol.h |2 +-
20 files changed, 64 insertions(+), 49 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-connection.c
b/libvirt-gobject/libvirt-gobject-connection.c
index 6c8de11..f52b825 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -1145,7 +1145,7 @@ G_DEFINE_BOXED_TYPE(GVirConnectionHandle,
gvir_connection_handle,
* Return value: (transfer full): a #GVirStream stream, or NULL
*/
GVirStream *gvir_connection_get_stream(GVirConnection *self,
- gint flags)
+ guint flags)
{
GVirConnectionClass *klass;
@@ -1212,7 +1212,7 @@ GVirDomain *gvir_connection_create_domain(GVirConnection
*conn,
GVirStoragePool *gvir_connection_create_storage_pool
(GVirConnection *conn,
GVirConfigStoragePool *conf,
- guint64 flags G_GNUC_UNUSED,
+ guint flags,
GError **err) {
const gchar *xml;
virStoragePoolPtr handle;
@@ -1222,7 +1222,7 @@ GVirStoragePool *gvir_connection_create_storage_pool
g_return_val_if_fail(xml != NULL, NULL);
-if (!(handle = virStoragePoolDefineXML(priv->conn, xml, 0))) {
+if (!(handle = virStoragePoolDefineXML(priv->conn, xml, flags))) {
*err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
flags,
"Failed to create storage pool");
diff --git a/libvirt-gobject/libvirt-gobject-connection.h
b/libvirt-gobject/libvirt-gobject-connection.h
index 9f4bdc3..92a15ab 100644
--- a/libvirt-gobject/libvirt-gobject-connection.h
+++ b/libvirt-gobject/libvirt-gobject-connection.h
@@ -163,12 +163,12 @@ GVirStoragePool
*gvir_connection_find_storage_pool_by_name(GVirConnection *conn,
GVirStoragePool *gvir_connection_create_storage_pool
(GVirConnection *conn,
GVirConfigStoragePool *conf,
- guint64 flags,
+ guint flags,
GError **err);
GVirStream *gvir_connection_get_stream(GVirConnection *conn,
- gint flags);
+ guint flags);
G_END_DECLS
diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
index 96be997..e536d72 100644
--- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
+++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
@@ -197,7 +197,7 @@ const gchar
*gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot)
*/
GVirConfigDomainSnapshot *gvir_domain_snapshot_get_config
(GVirDomainSnapshot *snapshot,
- guint64 flags,
+ guint flags,
GError **err)
{
GVirDomainSnapshotPrivate *priv = snapshot->priv;
diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.h
b/libvirt-gobject/libvirt-gobject-domain-snapshot.h
index 457c9dd..fc2eb7b 100644
--- a/libvirt-gobject/libvirt-gobject-domain-snapshot.h
+++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.h
@@ -66,7 +66,7 @@ const gchar *gvir_domain_snapshot_get_name(GVirDomainSnapshot
*snapshot);
GVirConfigDomainSnapshot *gvir_domain_snapshot_get_config
(GVirDomainSnapshot *snapshot,
- guint64 flags,
+ guint flags,
[libvirt] [PATCH libvirt-glib 4/5] Add API for creating transient domains
From: "Daniel P. Berrange"
---
libvirt-gobject/libvirt-gobject-connection.c | 49 ++
libvirt-gobject/libvirt-gobject-connection.h |4 ++
2 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-connection.c
b/libvirt-gobject/libvirt-gobject-connection.c
index f52b825..b6e7f3b 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -1164,6 +1164,10 @@ GVirStream *gvir_connection_get_stream(GVirConnection
*self,
* gvir_connection_create_domain:
* @conn: the connection on which to create the dmain
* @conf: the configuration for the new domain
+ *
+ * Create the configuration file for a new persistent domain.
+ * The returned domain will initially be in the shutoff state.
+ *
* Returns: (transfer full): the newly created domain
*/
GVirDomain *gvir_connection_create_domain(GVirConnection *conn,
@@ -1201,6 +1205,51 @@ GVirDomain *gvir_connection_create_domain(GVirConnection
*conn,
}
/**
+ * gvir_connection_start_domain:
+ * @conn: the connection on which to create the dmain
+ * @conf: the configuration for the new domain
+ *
+ * Start a new transient domain without persistent configuration.
+ * The returned domain will initially be running.
+ *
+ * Returns: (transfer full): the newly created domain
+ */
+GVirDomain *gvir_connection_start_domain(GVirConnection *conn,
+ GVirConfigDomain *conf,
+ guint flags,
+ GError **err)
+{
+const gchar *xml;
+virDomainPtr handle;
+GVirConnectionPrivate *priv = conn->priv;
+
+xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
+
+g_return_val_if_fail(xml != NULL, NULL);
+
+if (!(handle = virDomainCreateXML(priv->conn, xml, flags))) {
+*err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
+ 0,
+ "Failed to create domain");
+return NULL;
+}
+
+GVirDomain *domain;
+
+domain = GVIR_DOMAIN(g_object_new(GVIR_TYPE_DOMAIN,
+ "handle", handle,
+ NULL));
+
+g_mutex_lock(priv->lock);
+g_hash_table_insert(priv->domains,
+(gpointer)gvir_domain_get_uuid(domain),
+domain);
+g_mutex_unlock(priv->lock);
+
+return g_object_ref(domain);
+}
+
+/**
* gvir_connection_create_storage_pool:
* @conn: the connection on which to create the pool
* @conf: the configuration for the new storage pool
diff --git a/libvirt-gobject/libvirt-gobject-connection.h
b/libvirt-gobject/libvirt-gobject-connection.h
index 92a15ab..298009a 100644
--- a/libvirt-gobject/libvirt-gobject-connection.h
+++ b/libvirt-gobject/libvirt-gobject-connection.h
@@ -110,6 +110,10 @@ GVirDomain
*gvir_connection_find_domain_by_name(GVirConnection *conn,
GVirDomain *gvir_connection_create_domain(GVirConnection *conn,
GVirConfigDomain *conf,
GError **err);
+GVirDomain *gvir_connection_start_domain(GVirConnection *conn,
+ GVirConfigDomain *conf,
+ guint flags,
+ GError **err);
#if 0
GList *gvir_connection_get_interfaces(GVirConnection *conn);
--
1.7.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-glib 3/5] Uncomment & fix code for returning config objects
From: "Daniel P. Berrange" --- libvirt-gobject/libvirt-gobject-domain-snapshot.c |5 + libvirt-gobject/libvirt-gobject-interface.c |5 + libvirt-gobject/libvirt-gobject-network-filter.c |5 + libvirt-gobject/libvirt-gobject-network.c |5 + libvirt-gobject/libvirt-gobject-node-device.c |6 +- libvirt-gobject/libvirt-gobject-secret.c |6 +- libvirt-gobject/libvirt-gobject-storage-pool.c|5 + libvirt-gobject/libvirt-gobject-storage-vol.c |5 + 8 files changed, 8 insertions(+), 34 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c b/libvirt-gobject/libvirt-gobject-domain-snapshot.c index e536d72..c65a183 100644 --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c @@ -210,11 +210,8 @@ GVirConfigDomainSnapshot *gvir_domain_snapshot_get_config return NULL; } -#if 0 -GVirConfigDomainSnapshot *conf = gvir_config_domain_snapshot_new(xml); +GVirConfigDomainSnapshot *conf = gvir_config_domain_snapshot_new_from_xml(xml, err); g_free(xml); return conf; -#endif -return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-interface.c b/libvirt-gobject/libvirt-gobject-interface.c index d35cdc2..e47395c 100644 --- a/libvirt-gobject/libvirt-gobject-interface.c +++ b/libvirt-gobject/libvirt-gobject-interface.c @@ -199,11 +199,8 @@ GVirConfigInterface *gvir_interface_get_config(GVirInterface *iface, return NULL; } -#if 0 -GVirConfigInterface *conf = gvir_config_interface_new(xml); +GVirConfigInterface *conf = gvir_config_interface_new_from_xml(xml, err); g_free(xml); return conf; -#endif -return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c b/libvirt-gobject/libvirt-gobject-network-filter.c index 6ce0f7c..bdb0e3a 100644 --- a/libvirt-gobject/libvirt-gobject-network-filter.c +++ b/libvirt-gobject/libvirt-gobject-network-filter.c @@ -225,11 +225,8 @@ GVirConfigNetworkFilter *gvir_network_filter_get_config return NULL; } -#if 0 -GVirConfigNetworkFilter *conf = gvir_config_network_filter_new(xml); +GVirConfigNetworkFilter *conf = gvir_config_network_filter_new_from_xml(xml, err); g_free(xml); return conf; -#endif -return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-network.c b/libvirt-gobject/libvirt-gobject-network.c index 237f788..c486561 100644 --- a/libvirt-gobject/libvirt-gobject-network.c +++ b/libvirt-gobject/libvirt-gobject-network.c @@ -221,11 +221,8 @@ GVirConfigNetwork *gvir_network_get_config(GVirNetwork *network, return NULL; } -#if 0 -GVirConfigNetwork *conf = gvir_config_network_new(xml); +GVirConfigNetwork *conf = gvir_config_network_new_from_xml(xml, err); g_free(xml); return conf; -#endif -return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-node-device.c b/libvirt-gobject/libvirt-gobject-node-device.c index 162f930..43564b6 100644 --- a/libvirt-gobject/libvirt-gobject-node-device.c +++ b/libvirt-gobject/libvirt-gobject-node-device.c @@ -200,12 +200,8 @@ GVirConfigNodeDevice *gvir_node_device_get_config(GVirNodeDevice *device, return NULL; } -#if 0 -GVirConfigNodeDevice *conf = gvir_config_node_device_new(xml); +GVirConfigNodeDevice *conf = gvir_config_node_device_new_from_xml(xml, err); g_free(xml); return conf; -#endif - -return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-secret.c b/libvirt-gobject/libvirt-gobject-secret.c index 5bde345..418e5aa 100644 --- a/libvirt-gobject/libvirt-gobject-secret.c +++ b/libvirt-gobject/libvirt-gobject-secret.c @@ -211,12 +211,8 @@ GVirConfigSecret *gvir_secret_get_config(GVirSecret *secret, return NULL; } -#if 0 -GVirConfigSecret *conf = gvir_config_secret_new(xml); +GVirConfigSecret *conf = gvir_config_secret_new_from_xml(xml, err); g_free(xml); return conf; -#endif - -return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 915e0a1..92be539 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -236,13 +236,10 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool, return NULL; } -#if 0 -GVirConfigStoragePool *conf = gvir_config_storage_pool_new(xml); +GVirConfigStoragePool *conf = gvir_config_storage_pool_new_from_xml(xml, err); g_free(xml); return conf; -#endif -return NULL; } typedef gint (* CountFunction) (virStoragePoolPtr vpool); diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index a8aec60..17aac36 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -211,11 +211,8
[libvirt] [PATCH libvirt-glib 5/5] Ensure domains & pools hash tables in connection are non-NULL
From: "Daniel P. Berrange" --- libvirt-gobject/libvirt-gobject-connection.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index b6e7f3b..affb496 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -234,9 +234,15 @@ static void gvir_connection_init(GVirConnection *conn) priv = conn->priv = GVIR_CONNECTION_GET_PRIVATE(conn); -memset(priv, 0, sizeof(*priv)); - priv->lock = g_mutex_new(); +priv->domains = g_hash_table_new_full(g_str_hash, + g_str_equal, + NULL, + g_object_unref); +priv->pools = g_hash_table_new_full(g_str_hash, +g_str_equal, +NULL, +g_object_unref); } -- 1.7.6.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-glib 1/5] Add support for writing to streams
From: "Daniel P. Berrange"
---
libvirt-gobject/Makefile.am |8 +-
libvirt-gobject/libvirt-gobject-output-stream.c | 240 +++
libvirt-gobject/libvirt-gobject-output-stream.h | 68 +++
libvirt-gobject/libvirt-gobject-stream.c| 115 +++-
libvirt-gobject/libvirt-gobject-stream.h| 27 ++-
5 files changed, 447 insertions(+), 11 deletions(-)
create mode 100644 libvirt-gobject/libvirt-gobject-output-stream.c
create mode 100644 libvirt-gobject/libvirt-gobject-output-stream.h
diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am
index 0eef9c8..ec7b454 100644
--- a/libvirt-gobject/Makefile.am
+++ b/libvirt-gobject/Makefile.am
@@ -45,8 +45,7 @@ GOBJECT_GENERATED_FILES = \
libvirt_gobject_1_0_ladir = $(includedir)/libvirt-gobject-1.0/libvirt-gobject
libvirt_gobject_1_0_la_HEADERS = \
- $(GOBJECT_HEADER_FILES) \
- libvirt-gobject-input-stream.h
+ $(GOBJECT_HEADER_FILES)
nodist_libvirt_gobject_1_0_la_HEADERS = \
libvirt-gobject-enums.h
libvirt_gobject_1_0_la_SOURCES = \
@@ -54,7 +53,10 @@ libvirt_gobject_1_0_la_SOURCES = \
$(GOBJECT_SOURCE_FILES) \
libvirt-gobject-domain-device-private.h \
libvirt-gobject-compat.h \
- libvirt-gobject-input-stream.c
+ libvirt-gobject-input-stream.h \
+ libvirt-gobject-input-stream.c \
+ libvirt-gobject-output-stream.h \
+ libvirt-gobject-output-stream.c
nodist_libvirt_gobject_1_0_la_SOURCES = \
$(GOBJECT_GENERATED_FILES)
libvirt_gobject_1_0_la_CFLAGS = \
diff --git a/libvirt-gobject/libvirt-gobject-output-stream.c
b/libvirt-gobject/libvirt-gobject-output-stream.c
new file mode 100644
index 000..30ee519
--- /dev/null
+++ b/libvirt-gobject/libvirt-gobject-output-stream.c
@@ -0,0 +1,240 @@
+/*
+ * libvirt-gobject-output-stream.h: libvirt gobject integration
+ *
+ * Copyright (C) 2011 Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Authors: Daniel P. Berrange
+ * Marc-André Lureau
+ */
+
+#include
+
+#include
+#include
+
+#include "libvirt-glib/libvirt-glib.h"
+#include "libvirt-gobject/libvirt-gobject.h"
+#include "libvirt-gobject-output-stream.h"
+
+extern gboolean debugFlag;
+
+#define DEBUG(fmt, ...) do { if (G_UNLIKELY(debugFlag)) g_debug(fmt, ##
__VA_ARGS__); } while (0)
+
+#define gvir_output_stream_get_type _gvir_output_stream_get_type
+G_DEFINE_TYPE(GVirOutputStream, gvir_output_stream, G_TYPE_OUTPUT_STREAM);
+
+enum
+{
+PROP_0,
+PROP_STREAM
+};
+
+struct _GVirOutputStreamPrivate
+{
+GVirStream *stream;
+
+/* pending operation metadata */
+GSimpleAsyncResult *result;
+GCancellable *cancellable;
+const void * buffer;
+gsize count;
+};
+
+static void gvir_output_stream_get_property(GObject*object,
+guint prop_id,
+GValue *value,
+GParamSpec *pspec)
+{
+GVirOutputStream *stream = GVIR_OUTPUT_STREAM(object);
+
+switch (prop_id) {
+case PROP_STREAM:
+g_value_set_object(value, stream->priv->stream);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+static void gvir_output_stream_set_property(GObject *object,
+guint prop_id,
+const GValue *value,
+GParamSpec *pspec)
+{
+GVirOutputStream *stream = GVIR_OUTPUT_STREAM(object);
+
+switch (prop_id) {
+case PROP_STREAM:
+stream->priv->stream = g_value_get_object(value);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+static void gvir_output_stream_finalize(GObject *object)
+{
+GVirOutputStream *stream = GVIR_OUTPUT_STREAM(object);
+
+DEBUG("Finalize output stream GVirStream=%p", stream->priv->stream);
+stream->priv->stream = NUL
Re: [libvirt] [PATCH v3 1/2] Implement the asynchronous suspend and RTC wakeup
On 11/22/2011 06:18 AM, Eric Blake wrote: > On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote: >> Add the core functions that implement the functionality of the API. >> Suspend is done by using an asynchronous mechanism so that we can return >> the status to the caller successfully before the host gets suspended. This >> asynchronous operation is achieved by suspending the host in a separate >> thread of execution. >> >> To resume the host, an RTC alarm is set up (based on how long we want >> to suspend) before suspending the host. When this alarm fires, the host >> gets woken up. >> >> Signed-off-by: Srivatsa S. Bhat >> --- >> [...] >> + >> +#define MAX_SUSPEND_DURATION 365*24*60*60/* 1 year, a reasonable max? */ > > Any time you impose an arbitrary limit (and 1 year can be deemed > arbitrary), you are risking problems. There should be a real technical > reason why (and if) we have to impose a limit, not an arbitrary time > duration; otherwise, I would favor letting the user sleep as long as the > RTC clock supports (even if the sleep amount seems ridiculous to me). > > This is as far as I got on my review today; I'll pick up here tomorrow. > Hi Eric, Thanks a lot for the review comments! I'll address them in my next iteration of the patchset and post out soon. Thanks, Srivatsa S. Bhat IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-gconfig PATCHv2 04/32] Add gvir_config_object_replace_child helper
On Mon, Nov 21, 2011 at 07:04:01PM +0100, Christophe Fergeau wrote: > This allows us to factor the code to add an XML node to a config > object. > > -- > v2: use g_return_if_fail to test function args for sanity > > replace object > --- > libvirt-gconfig/libvirt-gconfig-object-private.h |5 + > libvirt-gconfig/libvirt-gconfig-object.c | 105 > +++--- > libvirt-gconfig/libvirt-gconfig.h|2 +- > 3 files changed, 80 insertions(+), 32 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 02/32] Remove unused prototype for gvir_config_object_parse
On Mon, Nov 21, 2011 at 07:03:59PM +0100, Christophe Fergeau wrote: > There is no corresponding implementation so no need to keep this > prototype. > --- > libvirt-gconfig/libvirt-gconfig-object.h |3 --- > 1 files changed, 0 insertions(+), 3 deletions(-) > > diff --git a/libvirt-gconfig/libvirt-gconfig-object.h > b/libvirt-gconfig/libvirt-gconfig-object.h > index 52e4525..bac3403 100644 > --- a/libvirt-gconfig/libvirt-gconfig-object.h > +++ b/libvirt-gconfig/libvirt-gconfig-object.h > @@ -85,9 +85,6 @@ void > gvir_config_object_set_node_content_uint64(GVirConfigObject *object, > const char *node_name, > guint64 value); > > -/* FIXME: move to a libvirt-gconfig-helpers.h file? */ > -xmlNodePtr gvir_config_object_parse(const char *xml, const char *root_node, > GError **err); > - > G_END_DECLS > > #endif /* __LIBVIRT_GCONFIG_OBJECT_H__ */ ACK 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] [libvirt-gconfig PATCHv2 05/32] Use new helpers to simplify gvir_config_domain_set_features
On Mon, Nov 21, 2011 at 07:04:02PM +0100, Christophe Fergeau wrote: > --- > libvirt-gconfig/libvirt-gconfig-domain.c | 17 +++-- > libvirt-gconfig/libvirt-gconfig-domain.h |1 - > 2 files changed, 3 insertions(+), 15 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 03/32] Make some GVirConfigObject helpers private
On Mon, Nov 21, 2011 at 07:04:00PM +0100, Christophe Fergeau wrote: > At this point, I'm not sure how/if the library user should be able > to directly manipulate the XML data wrapped by a GVirConfigObject. > It's preferrable to hide this API from the user until we have a > clearer idea how to expose it. > --- > libvirt-gconfig/Makefile.am |3 +- > libvirt-gconfig/libvirt-gconfig-domain.c |1 + > libvirt-gconfig/libvirt-gconfig-object-private.h | 41 > ++ > libvirt-gconfig/libvirt-gconfig-object.c | 28 +-- > libvirt-gconfig/libvirt-gconfig-object.h | 11 -- > libvirt-gconfig/libvirt-gconfig.sym |1 - > 6 files changed, 61 insertions(+), 24 deletions(-) > create mode 100644 libvirt-gconfig/libvirt-gconfig-object-private.h ACK 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 RESEND v5] Export KVM Host Power Management capabilities
On 11/22/2011 05:53 AM, Eric Blake wrote: > On 11/13/2011 10:29 PM, Srivatsa S. Bhat wrote: >> On 11/09/2011 04:38 PM, Srivatsa S. Bhat wrote: >>> This patch exports KVM Host Power Management capabilities as XML so that >>> higher-level systems management software can make use of these features >>> available in the host. >>> > >>> Changelog: >>> - >>> This version v5: >>> Some redundant error messages were removed and the code was streamlined. >>> >>> v4: http://www.redhat.com/archives/libvir-list/2011-August/msg00316.html >>> v3: http://www.redhat.com/archives/libvir-list/2011-August/msg00282.html >>> v2: http://www.redhat.com/archives/libvir-list/2011-August/msg00238.html >>> v1: http://thread.gmane.org/gmane.comp.emulators.libvirt/40886 >>> >>> References: >>> -- >>> [1] http://www.redhat.com/archives/libvir-list/2011-August/msg00248.html >>> [2] http://www.redhat.com/archives/libvir-list/2011-August/msg00302.html >>> >>> Signed-off-by: Srivatsa S. Bhat >> >> Any suggestions/comments on this patch? >> As suggested in one of the earlier versions, I have posted the follow-on >> patchset to add libvirt APIs to invoke suspend/resume in-band, here: >> http://www.redhat.com/archives/libvir-list/2011-November/msg00381.html > > Apologies for the delays, but I'm finally getting around to reviewing > this series, along with the v3 series to expose an API to control power > management on the host. I promise to either apply both series as a unit > in time for 0.9.8, or give further feedback on how to improve them in > the next revision, but I think we're converging on a useful design to > make it worth exposing this information. > Hi Eric, Thanks a lot for your time and for reviving this thread with your reviews and also for your reviews in the previous iterations of this patch! Regards, Srivatsa S. Bhat IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND v5] Export KVM Host Power Management capabilities
On 11/22/2011 08:59 AM, Daniel Veillard wrote: > On Wed, Nov 09, 2011 at 04:38:02PM +0530, Srivatsa S. Bhat wrote: >> This patch exports KVM Host Power Management capabilities as XML so that >> higher-level systems management software can make use of these features >> available in the host. >> >> The script "pm-is-supported" (from pm-utils package) is run to discover if >> Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. >> If either of them are supported, then a new tag "" is >> introduced in the XML under the tag. >> >> Eg: When the host supports both S3 and S4, the XML looks like this: >> [...] > > ACK, I made the couple of modifications above, cleaned up "make syntax-check" > and fixed a small issue in applying the patch to util.h, then commited > it, > > thanks a lot ! > Hi Daniel, Thank you for your review, ACK and for pushing the patch! Regards, Srivatsa S. Bhat IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-gconfig PATCHv2 01/32] Remove unneeded blank line in *_class_init
On Mon, Nov 21, 2011 at 07:03:58PM +0100, Christophe Fergeau wrote: > --- > libvirt-gconfig/libvirt-gconfig-capabilities.c|1 - > libvirt-gconfig/libvirt-gconfig-domain-snapshot.c |1 - > libvirt-gconfig/libvirt-gconfig-interface.c |1 - > libvirt-gconfig/libvirt-gconfig-network-filter.c |1 - > libvirt-gconfig/libvirt-gconfig-network.c |1 - > libvirt-gconfig/libvirt-gconfig-node-device.c |1 - > libvirt-gconfig/libvirt-gconfig-secret.c |1 - > libvirt-gconfig/libvirt-gconfig-storage-pool.c|1 - > libvirt-gconfig/libvirt-gconfig-storage-vol.c |1 - > 9 files changed, 0 insertions(+), 9 deletions(-) ACK, trivial 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] [libvirt-gconfig PATCHv2 06/32] Add gobject boilerplate for GVirConfigClock and GVirConfigTimer
On Mon, Nov 21, 2011 at 07:04:03PM +0100, Christophe Fergeau wrote: > --- > libvirt-gconfig/Makefile.am |4 ++ > libvirt-gconfig/libvirt-gconfig-clock.c | 81 > +++ > libvirt-gconfig/libvirt-gconfig-clock.h | 68 ++ > libvirt-gconfig/libvirt-gconfig-timer.c | 81 > +++ > libvirt-gconfig/libvirt-gconfig-timer.h | 68 ++ > libvirt-gconfig/libvirt-gconfig.h |2 + > libvirt-gconfig/libvirt-gconfig.sym |8 +++ > 7 files changed, 312 insertions(+), 0 deletions(-) > create mode 100644 libvirt-gconfig/libvirt-gconfig-clock.c > create mode 100644 libvirt-gconfig/libvirt-gconfig-clock.h > create mode 100644 libvirt-gconfig/libvirt-gconfig-timer.c > create mode 100644 libvirt-gconfig/libvirt-gconfig-timer.h ACK 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] [libvirt-gconfig PATCHv2 07/32] Add some GVirConfigClock setters
On Mon, Nov 21, 2011 at 07:04:04PM +0100, Christophe Fergeau wrote:
> --
> v2: use g_return_if_fail to test function args for sanity
> ---
> libvirt-gconfig/libvirt-gconfig-clock.c | 38
> +++
> libvirt-gconfig/libvirt-gconfig-clock.h |5
> libvirt-gconfig/libvirt-gconfig.sym |2 +
> 3 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/libvirt-gconfig/libvirt-gconfig-clock.c
> b/libvirt-gconfig/libvirt-gconfig-clock.c
> index 1f28efb..25f6159 100644
> --- a/libvirt-gconfig/libvirt-gconfig-clock.c
> +++ b/libvirt-gconfig/libvirt-gconfig-clock.c
> @@ -27,6 +27,7 @@
> #include
>
> #include "libvirt-gconfig/libvirt-gconfig.h"
> +#include "libvirt-gconfig/libvirt-gconfig-object-private.h"
>
> extern gboolean debugFlag;
>
> @@ -79,3 +80,40 @@ GVirConfigClock *gvir_config_clock_new_from_xml(const
> gchar *xml,
> "clock", NULL, xml, error);
> return GVIR_CONFIG_CLOCK(object);
> }
> +
> +void gvir_config_clock_set_timezone(GVirConfigClock *klock,
> +const char *tz)
> +{
> +xmlNodePtr node;
> +xmlChar *encoded_tz;
> +
> +g_return_if_fail(GVIR_IS_CONFIG_CLOCK(klock));
> +g_return_if_fail(tz != NULL);
> +
> +node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(klock));
> +if (node == NULL)
> +return;
> +
> +xmlNewProp(node, (xmlChar*)"offset", (xmlChar*)"timezone");
> +encoded_tz = xmlEncodeEntitiesReentrant(node->doc, (xmlChar*)tz);
> +xmlNewProp(node, (xmlChar*)"timezone", encoded_tz);
> +xmlFree(encoded_tz);
> +}
> +
> +void gvir_config_clock_set_variable_offset(GVirConfigClock *klock,
> + gint seconds)
> +{
> +xmlNodePtr node;
> +char *offset_str;
> +
> +g_return_if_fail(GVIR_IS_CONFIG_CLOCK(klock));
> +
> +node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(klock),
> "clock");
> +if (node == NULL)
> +return;
> +
> +xmlNewProp(node, (xmlChar*)"offset", (xmlChar*)"variable");
> +offset_str = g_strdup_printf("%d", seconds);
> +xmlNewProp(node, (xmlChar*)"timezone", (xmlChar*)offset_str);
> +g_free(offset_str);
Copy+paste buglet - s/timezone/adjustment/
ACK with that fix commit.
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] [libvirt-gconfig PATCHv2 09/32] Use glib-mkenums to register enums with glib
On Mon, Nov 21, 2011 at 07:04:06PM +0100, Christophe Fergeau wrote: > We don't currently have any enum in our API, but we will need some. > This commit adds the generation of libvirt-gconfig-enum-types.[ch] > using glib-mkenums. These files will register the various enums > that will get added to libvirt-gconfig header files with glib. > > -- > v2: move libvirt-gconfig-enum-types.h to HEADERS in Makefile.am so that > it's installed > --- > configure.ac |4 ++ > libvirt-gconfig/Makefile.am| 23 +++- > .../libvirt-gconfig-enum-types.c.template | 36 > > .../libvirt-gconfig-enum-types.h.template | 24 + > libvirt-gconfig/libvirt-gconfig.h |1 + > 5 files changed, 86 insertions(+), 2 deletions(-) > create mode 100644 libvirt-gconfig/libvirt-gconfig-enum-types.c.template > create mode 100644 libvirt-gconfig/libvirt-gconfig-enum-types.h.template ACK 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] [libvirt-gconfig PATCHv2 10/32] Add gvir_config_genum_get_nick helper
On Mon, Nov 21, 2011 at 07:04:07PM +0100, Christophe Fergeau wrote: > We will often need to convert from an enum to its string > representation, add an helper for that to avoid duplicating that > code. > > -- > v2: moved before gvir_config_clock_set_offset implementation since > it needs it > --- > libvirt-gconfig/libvirt-gconfig-helpers-private.h |1 + > libvirt-gconfig/libvirt-gconfig-helpers.c | 17 + > 2 files changed, 18 insertions(+), 0 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 11/32] Implement gvir_config_clock_set_offset
On Mon, Nov 21, 2011 at 07:04:08PM +0100, Christophe Fergeau wrote: > -- > v2: use gvir_config_genum_get_nick helper since it's now added before > this commit > use g_return_if_fail to test function args for sanity > --- > libvirt-gconfig/libvirt-gconfig-clock.c | 17 + > libvirt-gconfig/libvirt-gconfig-clock.h |8 > libvirt-gconfig/libvirt-gconfig.sym |2 ++ > 3 files changed, 27 insertions(+), 0 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 08/32] Add gvir_config_domain_set_clock
On Mon, Nov 21, 2011 at 07:04:05PM +0100, Christophe Fergeau wrote:
> The implementation is likely to need to be completed later. We
> might want to store pointers from GVirConfigDomain to the associated
> GVirConfigClock, from GVirConfigClock to the GVirConfigDomain that
> contains it. Since I'm not sure yet if they will be needed, I chose
> to keep the implementation simple.
>
> --
> v2: use g_return_if_fail to test function args for sanity
> ---
> libvirt-gconfig/libvirt-gconfig-domain.c | 12
> libvirt-gconfig/libvirt-gconfig-domain.h |2 ++
> libvirt-gconfig/libvirt-gconfig.sym |1 +
> 3 files changed, 15 insertions(+), 0 deletions(-)
ACK
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym
> b/libvirt-gconfig/libvirt-gconfig.sym
> index c3d46d5..7b346a1 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -13,6 +13,7 @@ LIBVIRT_GOBJECT_0.0.1 {
> gvir_config_domain_get_type;
> gvir_config_domain_new;
> gvir_config_domain_new_from_xml;
> + gvir_config_domain_set_clock;
> gvir_config_domain_get_features;
> gvir_config_domain_set_features;
> gvir_config_domain_get_memory;
We should start a LIBVIRT_GOBJECT_0.0.2 block for symbols now 0.0.1 is
released.
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] [libvirt-gconfig PATCHv2 12/32] Create clock object in domain creation test
On Mon, Nov 21, 2011 at 07:04:09PM +0100, Christophe Fergeau wrote: > --- > libvirt-gconfig/tests/test-domain-create.c |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 13/32] Add gobject boilerplate for GVirConfigOs
On Mon, Nov 21, 2011 at 07:04:10PM +0100, Christophe Fergeau wrote:
> ---
> libvirt-gconfig/Makefile.am |2 +
> libvirt-gconfig/libvirt-gconfig-os.c | 79
> ++
> libvirt-gconfig/libvirt-gconfig-os.h | 67
> libvirt-gconfig/libvirt-gconfig.h|3 +-
> libvirt-gconfig/libvirt-gconfig.sym |4 ++
> 5 files changed, 154 insertions(+), 1 deletions(-)
> create mode 100644 libvirt-gconfig/libvirt-gconfig-os.c
> create mode 100644 libvirt-gconfig/libvirt-gconfig-os.h
ACK
> +static void gvir_config_os_init(GVirConfigOs *os)
> +{
> +GVirConfigOsPrivate *priv;
> +
> +DEBUG("Init GVirConfigOs=%p", os);
> +
> +priv = os->priv = GVIR_CONFIG_OS_GET_PRIVATE(os);
> +
> +memset(priv, 0, sizeof(*priv));
> +}
Can drop the memset() since we previously figured out we don't need that.
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] [libvirt-gconfig PATCHv2 14/32] Add some GVirConfigOs setters
On Mon, Nov 21, 2011 at 07:04:11PM +0100, Christophe Fergeau wrote: > -- > v2: merged several related commits > use safer g_strcmp0 for string comparison > use g_return_if_fail to test function args for sanity > --- > libvirt-gconfig/libvirt-gconfig-os.c | 138 > ++ > libvirt-gconfig/libvirt-gconfig-os.h | 27 +++ > libvirt-gconfig/libvirt-gconfig.sym | 11 +++ > 3 files changed, 176 insertions(+), 0 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 15/32] Implement gvir_config_domain_set_os
On Mon, Nov 21, 2011 at 07:04:12PM +0100, Christophe Fergeau wrote: > -- > v2: use g_return_if_fail to test function args for sanity > --- > libvirt-gconfig/libvirt-gconfig-domain.c | 12 > libvirt-gconfig/libvirt-gconfig-domain.h |2 ++ > libvirt-gconfig/libvirt-gconfig.h|4 +++- > libvirt-gconfig/libvirt-gconfig.sym |1 + > 4 files changed, 18 insertions(+), 1 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 08/32] Add gvir_config_domain_set_clock
On Tue, Nov 22, 2011 at 12:56:02PM +, Daniel P. Berrange wrote:
> On Mon, Nov 21, 2011 at 07:04:05PM +0100, Christophe Fergeau wrote:
> > The implementation is likely to need to be completed later. We
> > might want to store pointers from GVirConfigDomain to the associated
> > GVirConfigClock, from GVirConfigClock to the GVirConfigDomain that
> > contains it. Since I'm not sure yet if they will be needed, I chose
> > to keep the implementation simple.
> >
> > --
> > v2: use g_return_if_fail to test function args for sanity
> > ---
> > libvirt-gconfig/libvirt-gconfig-domain.c | 12
> > libvirt-gconfig/libvirt-gconfig-domain.h |2 ++
> > libvirt-gconfig/libvirt-gconfig.sym |1 +
> > 3 files changed, 15 insertions(+), 0 deletions(-)
>
> ACK
>
> > diff --git a/libvirt-gconfig/libvirt-gconfig.sym
> > b/libvirt-gconfig/libvirt-gconfig.sym
> > index c3d46d5..7b346a1 100644
> > --- a/libvirt-gconfig/libvirt-gconfig.sym
> > +++ b/libvirt-gconfig/libvirt-gconfig.sym
> > @@ -13,6 +13,7 @@ LIBVIRT_GOBJECT_0.0.1 {
> > gvir_config_domain_get_type;
> > gvir_config_domain_new;
> > gvir_config_domain_new_from_xml;
> > + gvir_config_domain_set_clock;
> > gvir_config_domain_get_features;
> > gvir_config_domain_set_features;
> > gvir_config_domain_get_memory;
>
> We should start a LIBVIRT_GOBJECT_0.0.2 block for symbols now 0.0.1 is
> released.
On second thought, we could just rename the existing symbol version
block. Since we are explicitly not ABI stable, renaming the block
will force an application rebuild which is desirable for now
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] [libvirt-gconfig PATCHv2 16/32] Add test for GVirConfigOs
On Mon, Nov 21, 2011 at 07:04:13PM +0100, Christophe Fergeau wrote: > --- > libvirt-gconfig/tests/test-domain-create.c | 19 ++- > 1 files changed, 18 insertions(+), 1 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 17/32] Add GVirConfigDomain::vcpu
On Mon, Nov 21, 2011 at 07:04:14PM +0100, Christophe Fergeau wrote: > --- > libvirt-gconfig/libvirt-gconfig-domain.c | 29 > > libvirt-gconfig/libvirt-gconfig-domain.h |3 ++ > libvirt-gconfig/libvirt-gconfig.sym|2 + > libvirt-gconfig/tests/test-domain-create.c |1 + > 4 files changed, 35 insertions(+), 0 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 19/32] Add gobject boilerplate for GVirConfigDeviceDisk
On Mon, Nov 21, 2011 at 07:04:16PM +0100, Christophe Fergeau wrote:
> ---
> libvirt-gconfig/Makefile.am |2 +
> libvirt-gconfig/libvirt-gconfig-device-disk.c | 81
> +
> libvirt-gconfig/libvirt-gconfig-device-disk.h | 68 +
> libvirt-gconfig/libvirt-gconfig.h |1 +
> libvirt-gconfig/libvirt-gconfig.sym |4 +
> 5 files changed, 156 insertions(+), 0 deletions(-)
> create mode 100644 libvirt-gconfig/libvirt-gconfig-device-disk.c
> create mode 100644 libvirt-gconfig/libvirt-gconfig-device-disk.h
ACK
> +
> +static void gvir_config_device_disk_init(GVirConfigDeviceDisk *disk)
> +{
> +GVirConfigDeviceDiskPrivate *priv;
> +
> +DEBUG("Init GVirConfigDeviceDisk=%p", disk);
> +
> +priv = disk->priv = GVIR_CONFIG_DEVICE_DISK_GET_PRIVATE(disk);
> +
> +memset(priv, 0, sizeof(*priv));
> +}
Can kill off memset() when pushing
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] [libvirt-gconfig PATCHv2 18/32] Add gobject boilerplate for GVirConfigDevice
On Mon, Nov 21, 2011 at 07:04:15PM +0100, Christophe Fergeau wrote: > This is an abstract type which will be used as a base class for > all objects stored in the section of a domain. > --- > libvirt-gconfig/Makefile.am |2 + > libvirt-gconfig/libvirt-gconfig-device.c | 61 > libvirt-gconfig/libvirt-gconfig-device.h | 64 > ++ > libvirt-gconfig/libvirt-gconfig.h|1 + > libvirt-gconfig/libvirt-gconfig.sym |2 + > 5 files changed, 130 insertions(+), 0 deletions(-) > create mode 100644 libvirt-gconfig/libvirt-gconfig-device.c > create mode 100644 libvirt-gconfig/libvirt-gconfig-device.h ACK 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 libvirt-glib 4/5] Add API for creating transient domains
Hey,
On Tue, Nov 22, 2011 at 12:39:31PM +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> ---
> libvirt-gobject/libvirt-gobject-connection.c | 49
> ++
> libvirt-gobject/libvirt-gobject-connection.h |4 ++
No addition of the new function in libvirt-gobject.sym?
> 2 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c
> b/libvirt-gobject/libvirt-gobject-connection.c
> index f52b825..b6e7f3b 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -1164,6 +1164,10 @@ GVirStream *gvir_connection_get_stream(GVirConnection
> *self,
> * gvir_connection_create_domain:
> * @conn: the connection on which to create the dmain
domain
> * @conf: the configuration for the new domain
> + *
> + * Create the configuration file for a new persistent domain.
> + * The returned domain will initially be in the shutoff state.
> + *
> * Returns: (transfer full): the newly created domain
> */
> GVirDomain *gvir_connection_create_domain(GVirConnection *conn,
> @@ -1201,6 +1205,51 @@ GVirDomain
> *gvir_connection_create_domain(GVirConnection *conn,
> }
>
> /**
> + * gvir_connection_start_domain:
> + * @conn: the connection on which to create the dmain
Same here, domain
> + * @conf: the configuration for the new domain
> + *
> + * Start a new transient domain without persistent configuration.
> + * The returned domain will initially be running.
> + *
> + * Returns: (transfer full): the newly created domain
> + */
> +GVirDomain *gvir_connection_start_domain(GVirConnection *conn,
> + GVirConfigDomain *conf,
> + guint flags,
> + GError **err)
> +{
> +const gchar *xml;
> +virDomainPtr handle;
> +GVirConnectionPrivate *priv = conn->priv;
> +
> +xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
xml shouldn't be const and need to be g_freed when it's no longer needed.
> +
> +g_return_val_if_fail(xml != NULL, NULL);
> +
> +if (!(handle = virDomainCreateXML(priv->conn, xml, flags))) {
> +*err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
> + 0,
> + "Failed to create domain");
> +return NULL;
> +}
> +
> +GVirDomain *domain;
> +
> +domain = GVIR_DOMAIN(g_object_new(GVIR_TYPE_DOMAIN,
> + "handle", handle,
> + NULL));
> +
> +g_mutex_lock(priv->lock);
> +g_hash_table_insert(priv->domains,
> +(gpointer)gvir_domain_get_uuid(domain),
> +domain);
> +g_mutex_unlock(priv->lock);
> +
> +return g_object_ref(domain);
Here I'd do
g_mutex_lock(priv->lock);
g_hash_table_insert(priv->domains,
(gpointer)gvir_domain_get_uuid(domain),
g_object_ref(domain));
g_mutex_unlock(priv->lock);
return domain;
to make it clearer that the hash table needs a ref on the object. The way
it's written, I had to check how priv->domains is declared to make sure
that wasn't an extra _ref.
Christophe
> +}
> +
> +/**
> * gvir_connection_create_storage_pool:
> * @conn: the connection on which to create the pool
> * @conf: the configuration for the new storage pool
> diff --git a/libvirt-gobject/libvirt-gobject-connection.h
> b/libvirt-gobject/libvirt-gobject-connection.h
> index 92a15ab..298009a 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.h
> +++ b/libvirt-gobject/libvirt-gobject-connection.h
> @@ -110,6 +110,10 @@ GVirDomain
> *gvir_connection_find_domain_by_name(GVirConnection *conn,
> GVirDomain *gvir_connection_create_domain(GVirConnection *conn,
>GVirConfigDomain *conf,
>GError **err);
> +GVirDomain *gvir_connection_start_domain(GVirConnection *conn,
> + GVirConfigDomain *conf,
> + guint flags,
> + GError **err);
>
> #if 0
> GList *gvir_connection_get_interfaces(GVirConnection *conn);
> --
> 1.7.6.4
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
pgpaOvBqQRHoE.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-gconfig PATCHv2 20/32] Add various GVirConfigDeviceDisk setters
On Mon, Nov 21, 2011 at 07:04:17PM +0100, Christophe Fergeau wrote: > -- > v2: use g_return_if_fail to test function args for sanity > --- > libvirt-gconfig/libvirt-gconfig-device-disk.c | 50 > + > libvirt-gconfig/libvirt-gconfig-device-disk.h | 25 > libvirt-gconfig/libvirt-gconfig.sym |6 +++ > 3 files changed, 81 insertions(+), 0 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 21/32] Add gvir_config_domain_set_devices
On Mon, Nov 21, 2011 at 07:04:18PM +0100, Christophe Fergeau wrote: > -- > v2: use g_return_if_fail to test function args for sanity > --- > libvirt-gconfig/libvirt-gconfig-domain.c | 23 +++ > libvirt-gconfig/libvirt-gconfig-domain.h |2 ++ > libvirt-gconfig/libvirt-gconfig.sym |3 ++- > 3 files changed, 27 insertions(+), 1 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 23/32] More GVirConfigDeviceDisk setters
On Mon, Nov 21, 2011 at 07:04:20PM +0100, Christophe Fergeau wrote: > -- > v2: fix gvir_config_device_disk_set_source > use g_return_if_fail to test function args for sanity > --- > libvirt-gconfig/libvirt-gconfig-device-disk.c | 78 > - > libvirt-gconfig/libvirt-gconfig-device-disk.h | 10 +++ > libvirt-gconfig/libvirt-gconfig.sym |7 ++- > 3 files changed, 93 insertions(+), 2 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 22/32] Add gvir_config_object_add_child
On Mon, Nov 21, 2011 at 07:04:19PM +0100, Christophe Fergeau wrote: > It's similar to gvir_config_object_replace_child except that if the > current node already has a child with the correct name, it returns > the existing child instead of replacing it. > > -- > v2: instead of adding an argument to gvir_config_object_new_child, > split the function in 2 separate ones, gvir_config_object_add_child > and gvir_config_object_replace_child. > use g_return_if_fail to test function args for sanity ACK 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 libvirt-glib 2/5] Change all flags from guint64 to guint to match libvirt type
ack
On Tue, Nov 22, 2011 at 12:39:29PM +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> ---
> libvirt-gobject/libvirt-gobject-connection.c |6 ++--
> libvirt-gobject/libvirt-gobject-connection.h |4 +-
> libvirt-gobject/libvirt-gobject-domain-snapshot.c |2 +-
> libvirt-gobject/libvirt-gobject-domain-snapshot.h |2 +-
> libvirt-gobject/libvirt-gobject-domain.c | 37 ++--
> libvirt-gobject/libvirt-gobject-domain.h | 16
> libvirt-gobject/libvirt-gobject-interface.c |2 +-
> libvirt-gobject/libvirt-gobject-interface.h |2 +-
> libvirt-gobject/libvirt-gobject-network-filter.c |2 +-
> libvirt-gobject/libvirt-gobject-network-filter.h |2 +-
> libvirt-gobject/libvirt-gobject-network.c |2 +-
> libvirt-gobject/libvirt-gobject-network.h |2 +-
> libvirt-gobject/libvirt-gobject-node-device.c |2 +-
> libvirt-gobject/libvirt-gobject-node-device.h |2 +-
> libvirt-gobject/libvirt-gobject-secret.c |2 +-
> libvirt-gobject/libvirt-gobject-secret.h |2 +-
> libvirt-gobject/libvirt-gobject-storage-pool.c| 12 +++---
> libvirt-gobject/libvirt-gobject-storage-pool.h| 10 +++---
> libvirt-gobject/libvirt-gobject-storage-vol.c |2 +-
> libvirt-gobject/libvirt-gobject-storage-vol.h |2 +-
> 20 files changed, 64 insertions(+), 49 deletions(-)
>
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c
> b/libvirt-gobject/libvirt-gobject-connection.c
> index 6c8de11..f52b825 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -1145,7 +1145,7 @@ G_DEFINE_BOXED_TYPE(GVirConnectionHandle,
> gvir_connection_handle,
> * Return value: (transfer full): a #GVirStream stream, or NULL
> */
> GVirStream *gvir_connection_get_stream(GVirConnection *self,
> - gint flags)
> + guint flags)
> {
> GVirConnectionClass *klass;
>
> @@ -1212,7 +1212,7 @@ GVirDomain
> *gvir_connection_create_domain(GVirConnection *conn,
> GVirStoragePool *gvir_connection_create_storage_pool
> (GVirConnection *conn,
> GVirConfigStoragePool *conf,
> - guint64 flags G_GNUC_UNUSED,
> + guint flags,
> GError **err) {
> const gchar *xml;
> virStoragePoolPtr handle;
> @@ -1222,7 +1222,7 @@ GVirStoragePool *gvir_connection_create_storage_pool
>
> g_return_val_if_fail(xml != NULL, NULL);
>
> -if (!(handle = virStoragePoolDefineXML(priv->conn, xml, 0))) {
> +if (!(handle = virStoragePoolDefineXML(priv->conn, xml, flags))) {
> *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
>flags,
>"Failed to create storage pool");
> diff --git a/libvirt-gobject/libvirt-gobject-connection.h
> b/libvirt-gobject/libvirt-gobject-connection.h
> index 9f4bdc3..92a15ab 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.h
> +++ b/libvirt-gobject/libvirt-gobject-connection.h
> @@ -163,12 +163,12 @@ GVirStoragePool
> *gvir_connection_find_storage_pool_by_name(GVirConnection *conn,
> GVirStoragePool *gvir_connection_create_storage_pool
> (GVirConnection *conn,
> GVirConfigStoragePool *conf,
> - guint64 flags,
> + guint flags,
> GError **err);
>
>
> GVirStream *gvir_connection_get_stream(GVirConnection *conn,
> - gint flags);
> + guint flags);
>
> G_END_DECLS
>
> diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
> b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
> index 96be997..e536d72 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
> @@ -197,7 +197,7 @@ const gchar
> *gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot)
> */
> GVirConfigDomainSnapshot *gvir_domain_snapshot_get_config
> (GVirDomainSnapshot *snapshot,
> - guint64 flags,
> + guint flags,
> GError **err)
> {
> GVirDomainSnapshotPrivate *priv = snapshot->priv;
> diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.h
> b/libvirt-gobject/libvirt-gobject-domain-snapshot.h
> index 457c9dd..fc2eb7b 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.h
> +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.h
> @@ -66,7 +66,7 @@ const gchar
> *gvir_domain_snapshot_get_name(GVirDom
Re: [libvirt] [libvirt-gconfig PATCHv2 24/32] Add test for adding a disk device
On Mon, Nov 21, 2011 at 07:04:21PM +0100, Christophe Fergeau wrote: > --- > libvirt-gconfig/tests/test-domain-create.c | 19 +++ > 1 files changed, 19 insertions(+), 0 deletions(-) ACK 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] [libvirt-gconfig PATCHv2 25/32] GVirConfigInterface derives from GVirConfigDevice
On Mon, Nov 21, 2011 at 07:04:22PM +0100, Christophe Fergeau wrote: > This base class is mainly useful as a generic type when we manipulate > list of devices regardless of their actual type. > --- > libvirt-gconfig/libvirt-gconfig-interface.c |2 +- > libvirt-gconfig/libvirt-gconfig-interface.h |4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Hmm, I don't believe this is correct. GVirConfigInterface is a top level XML document used for the GVirInterface object in libvirt. NB, not to be confused with the GVirConfigDomainInterface which is a device element within the GVirConfigDomain XML document. So NACK to this 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] [libvirt-gconfig PATCHv2 32/32] Add GVirConfigDeviceVideo
On Mon, Nov 21, 2011 at 07:04:29PM +0100, Christophe Fergeau wrote: > -- > v2: use g_return_if_fail to test function args for sanity > --- > libvirt-gconfig/Makefile.am|2 + > libvirt-gconfig/libvirt-gconfig-device-video.c | 133 > > libvirt-gconfig/libvirt-gconfig-device-video.h | 82 +++ > libvirt-gconfig/libvirt-gconfig.h |1 + > libvirt-gconfig/libvirt-gconfig.sym|8 ++ > libvirt-gconfig/tests/test-domain-create.c |9 ++ > 6 files changed, 235 insertions(+), 0 deletions(-) > create mode 100644 libvirt-gconfig/libvirt-gconfig-device-video.c > create mode 100644 libvirt-gconfig/libvirt-gconfig-device-video.h > > diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am > index 1be6233..05cdbd0 100644 > --- a/libvirt-gconfig/Makefile.am > +++ b/libvirt-gconfig/Makefile.am > @@ -17,6 +17,7 @@ GCONFIG_HEADER_FILES = \ > libvirt-gconfig-device-graphics.h \ > libvirt-gconfig-device-graphics-spice.h \ > libvirt-gconfig-device-input.h \ > + libvirt-gconfig-device-video.h \ > libvirt-gconfig-domain.h \ > libvirt-gconfig-domain-snapshot.h \ > libvirt-gconfig-helpers.h \ > @@ -42,6 +43,7 @@ GCONFIG_SOURCE_FILES = \ > libvirt-gconfig-device-graphics.c \ > libvirt-gconfig-device-graphics-spice.c \ > libvirt-gconfig-device-input.c \ > + libvirt-gconfig-device-video.c \ > libvirt-gconfig-domain.c \ > libvirt-gconfig-domain-snapshot.c \ > libvirt-gconfig-enum-types.c \ Looking again at the earlier patches, we have a naming issue here. All the objects which are used as children of GVirConfigDomain, should share the same naming prefix, eg GVirConfigDomainVideo, GVirConfigDomainGraphics, GVirConfigDomainClock, etc, otherwise we will end up having significant namespace clashes with elements from other XML documents in libvirt. 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] [libvirt-gconfig PATCHv2 06/32] Add gobject boilerplate for GVirConfigClock and GVirConfigTimer
On Tue, Nov 22, 2011 at 12:48:54PM +, Daniel P. Berrange wrote: > On Mon, Nov 21, 2011 at 07:04:03PM +0100, Christophe Fergeau wrote: > > --- > > libvirt-gconfig/Makefile.am |4 ++ > > libvirt-gconfig/libvirt-gconfig-clock.c | 81 > > +++ > > libvirt-gconfig/libvirt-gconfig-clock.h | 68 ++ > > libvirt-gconfig/libvirt-gconfig-timer.c | 81 > > +++ > > libvirt-gconfig/libvirt-gconfig-timer.h | 68 ++ > > libvirt-gconfig/libvirt-gconfig.h |2 + > > libvirt-gconfig/libvirt-gconfig.sym |8 +++ > > 7 files changed, 312 insertions(+), 0 deletions(-) > > create mode 100644 libvirt-gconfig/libvirt-gconfig-clock.c > > create mode 100644 libvirt-gconfig/libvirt-gconfig-clock.h > > create mode 100644 libvirt-gconfig/libvirt-gconfig-timer.c > > create mode 100644 libvirt-gconfig/libvirt-gconfig-timer.h > > ACK Actually, I'm afraid these classes need renaming to GVirConfigDomainClock and GVirConfigDomainTimer. 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 libvirt-glib 3/5] Uncomment & fix code for returning config objects
Hi, On Tue, Nov 22, 2011 at 12:39:30PM +, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > --- > libvirt-gobject/libvirt-gobject-domain-snapshot.c |5 + > libvirt-gobject/libvirt-gobject-interface.c |5 + > libvirt-gobject/libvirt-gobject-network-filter.c |5 + > libvirt-gobject/libvirt-gobject-network.c |5 + > libvirt-gobject/libvirt-gobject-node-device.c |6 +- > libvirt-gobject/libvirt-gobject-secret.c |6 +- > libvirt-gobject/libvirt-gobject-storage-pool.c|5 + > libvirt-gobject/libvirt-gobject-storage-vol.c |5 + > 8 files changed, 8 insertions(+), 34 deletions(-) > > diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c > b/libvirt-gobject/libvirt-gobject-domain-snapshot.c > index e536d72..c65a183 100644 > --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c > +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c > @@ -210,11 +210,8 @@ GVirConfigDomainSnapshot *gvir_domain_snapshot_get_config > return NULL; > } > > -#if 0 > -GVirConfigDomainSnapshot *conf = gvir_config_domain_snapshot_new(xml); > +GVirConfigDomainSnapshot *conf = > gvir_config_domain_snapshot_new_from_xml(xml, err); > > g_free(xml); > return conf; The xml data comes from libvirt, so it should be freed with free(), not g_free. The same comment applies to all the other hunks. Christophe > -#endif > -return NULL; > } > diff --git a/libvirt-gobject/libvirt-gobject-interface.c > b/libvirt-gobject/libvirt-gobject-interface.c > index d35cdc2..e47395c 100644 > --- a/libvirt-gobject/libvirt-gobject-interface.c > +++ b/libvirt-gobject/libvirt-gobject-interface.c > @@ -199,11 +199,8 @@ GVirConfigInterface > *gvir_interface_get_config(GVirInterface *iface, > return NULL; > } > > -#if 0 > -GVirConfigInterface *conf = gvir_config_interface_new(xml); > +GVirConfigInterface *conf = gvir_config_interface_new_from_xml(xml, err); > > g_free(xml); > return conf; > -#endif > -return NULL; > } > diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c > b/libvirt-gobject/libvirt-gobject-network-filter.c > index 6ce0f7c..bdb0e3a 100644 > --- a/libvirt-gobject/libvirt-gobject-network-filter.c > +++ b/libvirt-gobject/libvirt-gobject-network-filter.c > @@ -225,11 +225,8 @@ GVirConfigNetworkFilter *gvir_network_filter_get_config > return NULL; > } > > -#if 0 > -GVirConfigNetworkFilter *conf = gvir_config_network_filter_new(xml); > +GVirConfigNetworkFilter *conf = > gvir_config_network_filter_new_from_xml(xml, err); > > g_free(xml); > return conf; > -#endif > -return NULL; > } > diff --git a/libvirt-gobject/libvirt-gobject-network.c > b/libvirt-gobject/libvirt-gobject-network.c > index 237f788..c486561 100644 > --- a/libvirt-gobject/libvirt-gobject-network.c > +++ b/libvirt-gobject/libvirt-gobject-network.c > @@ -221,11 +221,8 @@ GVirConfigNetwork *gvir_network_get_config(GVirNetwork > *network, > return NULL; > } > > -#if 0 > -GVirConfigNetwork *conf = gvir_config_network_new(xml); > +GVirConfigNetwork *conf = gvir_config_network_new_from_xml(xml, err); > > g_free(xml); > return conf; > -#endif > -return NULL; > } > diff --git a/libvirt-gobject/libvirt-gobject-node-device.c > b/libvirt-gobject/libvirt-gobject-node-device.c > index 162f930..43564b6 100644 > --- a/libvirt-gobject/libvirt-gobject-node-device.c > +++ b/libvirt-gobject/libvirt-gobject-node-device.c > @@ -200,12 +200,8 @@ GVirConfigNodeDevice > *gvir_node_device_get_config(GVirNodeDevice *device, > return NULL; > } > > -#if 0 > -GVirConfigNodeDevice *conf = gvir_config_node_device_new(xml); > +GVirConfigNodeDevice *conf = gvir_config_node_device_new_from_xml(xml, > err); > > g_free(xml); > return conf; > -#endif > - > -return NULL; > } > diff --git a/libvirt-gobject/libvirt-gobject-secret.c > b/libvirt-gobject/libvirt-gobject-secret.c > index 5bde345..418e5aa 100644 > --- a/libvirt-gobject/libvirt-gobject-secret.c > +++ b/libvirt-gobject/libvirt-gobject-secret.c > @@ -211,12 +211,8 @@ GVirConfigSecret *gvir_secret_get_config(GVirSecret > *secret, > return NULL; > } > > -#if 0 > -GVirConfigSecret *conf = gvir_config_secret_new(xml); > +GVirConfigSecret *conf = gvir_config_secret_new_from_xml(xml, err); > > g_free(xml); > return conf; > -#endif > - > -return NULL; > } > diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c > b/libvirt-gobject/libvirt-gobject-storage-pool.c > index 915e0a1..92be539 100644 > --- a/libvirt-gobject/libvirt-gobject-storage-pool.c > +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c > @@ -236,13 +236,10 @@ GVirConfigStoragePool > *gvir_storage_pool_get_config(GVirStoragePool *pool, > return NULL; > } > > -#if 0 > -GVirConfigStoragePool *conf = gvir_co
Re: [libvirt] [libvirt-gconfig PATCHv2 18/32] Add gobject boilerplate for GVirConfigDevice
On Tue, Nov 22, 2011 at 01:06:11PM +, Daniel P. Berrange wrote: > On Mon, Nov 21, 2011 at 07:04:15PM +0100, Christophe Fergeau wrote: > > This is an abstract type which will be used as a base class for > > all objects stored in the section of a domain. > > --- > > libvirt-gconfig/Makefile.am |2 + > > libvirt-gconfig/libvirt-gconfig-device.c | 61 > > > > libvirt-gconfig/libvirt-gconfig-device.h | 64 > > ++ > > libvirt-gconfig/libvirt-gconfig.h|1 + > > libvirt-gconfig/libvirt-gconfig.sym |2 + > > 5 files changed, 130 insertions(+), 0 deletions(-) > > create mode 100644 libvirt-gconfig/libvirt-gconfig-device.c > > create mode 100644 libvirt-gconfig/libvirt-gconfig-device.h > > ACK Sorry, needs renaming to GVirConfigDomainDevice 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 libvirt-glib 5/5] Ensure domains & pools hash tables in connection are non-NULL
Hi, I assume without this, it's easy to try to add data to NULL hash tables when using the transient domain API you added in this patch series? ACK patch, just curious... Christophe On Tue, Nov 22, 2011 at 12:39:32PM +, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > --- > libvirt-gobject/libvirt-gobject-connection.c | 10 -- > 1 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libvirt-gobject/libvirt-gobject-connection.c > b/libvirt-gobject/libvirt-gobject-connection.c > index b6e7f3b..affb496 100644 > --- a/libvirt-gobject/libvirt-gobject-connection.c > +++ b/libvirt-gobject/libvirt-gobject-connection.c > @@ -234,9 +234,15 @@ static void gvir_connection_init(GVirConnection *conn) > > priv = conn->priv = GVIR_CONNECTION_GET_PRIVATE(conn); > > -memset(priv, 0, sizeof(*priv)); > - > priv->lock = g_mutex_new(); > +priv->domains = g_hash_table_new_full(g_str_hash, > + g_str_equal, > + NULL, > + g_object_unref); > +priv->pools = g_hash_table_new_full(g_str_hash, > +g_str_equal, > +NULL, > +g_object_unref); > } > > > -- > 1.7.6.4 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list pgpD61n91dJti.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-gconfig PATCHv2 13/32] Add gobject boilerplate for GVirConfigOs
On Tue, Nov 22, 2011 at 01:01:17PM +, Daniel P. Berrange wrote: > On Mon, Nov 21, 2011 at 07:04:10PM +0100, Christophe Fergeau wrote: > > --- > > libvirt-gconfig/Makefile.am |2 + > > libvirt-gconfig/libvirt-gconfig-os.c | 79 > > ++ > > libvirt-gconfig/libvirt-gconfig-os.h | 67 > > libvirt-gconfig/libvirt-gconfig.h|3 +- > > libvirt-gconfig/libvirt-gconfig.sym |4 ++ > > 5 files changed, 154 insertions(+), 1 deletions(-) > > create mode 100644 libvirt-gconfig/libvirt-gconfig-os.c > > create mode 100644 libvirt-gconfig/libvirt-gconfig-os.h > > ACK Sorry, needs renaming to GVirConfigDomainOs. 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] [libvirt-glib] Add API to redefine an existing domain
On Tue, Nov 22, 2011 at 11:20 AM, Daniel P. Berrange
wrote:
> On Mon, Nov 21, 2011 at 08:27:17PM +0100, Marc-André Lureau wrote:
>> Hi
>>
>> On Mon, Nov 21, 2011 at 6:53 PM, Zeeshan Ali (Khattak)
>> wrote:
>> > + g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
>>
>> This is wrong, it should be error != NULL && *error == NULL.
>>
>> > + if (virDomainDefineXML(conn, xml) == NULL) {
>> > + if (error != NULL)
>> > + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
>> > + 0,
>> > + "Failed to set "
>> > + "domain configuration");
>> > + return FALSE;
>> > + }
>>
>> Can you please verify that the return value is safe to ignore?
>>
>> I would really prefer we add a runtime check that verifiy the handle
>> is the same as the one currently associated with the domain.
>
> The actual pointer returned will *not* be the same as the same
> one you currently have, but assuming the XML has matching UUID
> and name, it will point to the same domain.
>
> So I guess what you want todo is verify the UUID and name in
> the XML, before defining it.
>
> Also, you need to call virDomainFree on the return value of
> virDomainDefineXML to avoid memleak.
Thanks! I already pushed the patch that does this and Marc-Andre ack'ed.
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-gconfig PATCHv2 19/32] Add gobject boilerplate for GVirConfigDeviceDisk
On Tue, Nov 22, 2011 at 01:06:52PM +, Daniel P. Berrange wrote: > On Mon, Nov 21, 2011 at 07:04:16PM +0100, Christophe Fergeau wrote: > > --- > > libvirt-gconfig/Makefile.am |2 + > > libvirt-gconfig/libvirt-gconfig-device-disk.c | 81 > > + > > libvirt-gconfig/libvirt-gconfig-device-disk.h | 68 + > > libvirt-gconfig/libvirt-gconfig.h |1 + > > libvirt-gconfig/libvirt-gconfig.sym |4 + > > 5 files changed, 156 insertions(+), 0 deletions(-) > > create mode 100644 libvirt-gconfig/libvirt-gconfig-device-disk.c > > create mode 100644 libvirt-gconfig/libvirt-gconfig-device-disk.h > > ACK Sorry, need to rename GVirConfigDeviceDisk to GVirConfigDomainDisk. (I think also having Device in the name would be overkill, given that we're adding Domain into the name, and 'Disk' is unique enough within the scope of the Domain XML document) 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 libvirt-glib 5/5] Ensure domains & pools hash tables in connection are non-NULL
On Tue, Nov 22, 2011 at 02:22:40PM +0100, Christophe Fergeau wrote:
> Hi,
>
> I assume without this, it's easy to try to add data to NULL hash tables
> when using the transient domain API you added in this patch series?
> ACK patch, just curious...
Actually the scenario was with persistent domains too
conn = LibvirtGObject.Connection.new("qemu:///session")
conn.open(None)
conn.create_domain(domcfg)
which would SEGV unless you had called conn.fetch_domains before
conn.create_domain.
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 libvirt-glib 4/5] Add API for creating transient domains
On Tue, Nov 22, 2011 at 02:07:23PM +0100, Christophe Fergeau wrote:
> Hey,
>
> On Tue, Nov 22, 2011 at 12:39:31PM +, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange"
> >
> > ---
> > libvirt-gobject/libvirt-gobject-connection.c | 49
> > ++
> > libvirt-gobject/libvirt-gobject-connection.h |4 ++
>
> No addition of the new function in libvirt-gobject.sym?
>
> > 2 files changed, 53 insertions(+), 0 deletions(-)
> >
> > diff --git a/libvirt-gobject/libvirt-gobject-connection.c
> > b/libvirt-gobject/libvirt-gobject-connection.c
> > index f52b825..b6e7f3b 100644
> > --- a/libvirt-gobject/libvirt-gobject-connection.c
> > +++ b/libvirt-gobject/libvirt-gobject-connection.c
> > @@ -1164,6 +1164,10 @@ GVirStream
> > *gvir_connection_get_stream(GVirConnection *self,
> > * gvir_connection_create_domain:
> > * @conn: the connection on which to create the dmain
>
> domain
>
> > * @conf: the configuration for the new domain
> > + *
> > + * Create the configuration file for a new persistent domain.
> > + * The returned domain will initially be in the shutoff state.
> > + *
> > * Returns: (transfer full): the newly created domain
> > */
> > GVirDomain *gvir_connection_create_domain(GVirConnection *conn,
> > @@ -1201,6 +1205,51 @@ GVirDomain
> > *gvir_connection_create_domain(GVirConnection *conn,
> > }
> >
> > /**
> > + * gvir_connection_start_domain:
> > + * @conn: the connection on which to create the dmain
>
> Same here, domain
Cut + paste typos :-)
>
> > + * @conf: the configuration for the new domain
> > + *
> > + * Start a new transient domain without persistent configuration.
> > + * The returned domain will initially be running.
> > + *
> > + * Returns: (transfer full): the newly created domain
> > + */
> > +GVirDomain *gvir_connection_start_domain(GVirConnection *conn,
> > + GVirConfigDomain *conf,
> > + guint flags,
> > + GError **err)
> > +{
> > +const gchar *xml;
> > +virDomainPtr handle;
> > +GVirConnectionPrivate *priv = conn->priv;
> > +
> > +xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
>
> xml shouldn't be const and need to be g_freed when it's no longer needed.
Oooh, gvir_connection_create_domain has the same flaw. I'll fix
both.
> > +
> > +g_return_val_if_fail(xml != NULL, NULL);
> > +
> > +if (!(handle = virDomainCreateXML(priv->conn, xml, flags))) {
> > +*err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
> > + 0,
> > + "Failed to create domain");
> > +return NULL;
> > +}
> > +
> > +GVirDomain *domain;
> > +
> > +domain = GVIR_DOMAIN(g_object_new(GVIR_TYPE_DOMAIN,
> > + "handle", handle,
> > + NULL));
> > +
> > +g_mutex_lock(priv->lock);
> > +g_hash_table_insert(priv->domains,
> > +(gpointer)gvir_domain_get_uuid(domain),
> > +domain);
> > +g_mutex_unlock(priv->lock);
> > +
> > +return g_object_ref(domain);
>
> Here I'd do
>g_mutex_lock(priv->lock);
>g_hash_table_insert(priv->domains,
>(gpointer)gvir_domain_get_uuid(domain),
>g_object_ref(domain));
>g_mutex_unlock(priv->lock);
>
>return domain;
>
> to make it clearer that the hash table needs a ref on the object. The way
> it's written, I had to check how priv->domains is declared to make sure
> that wasn't an extra _ref.
Again this was a cut+past from gvir_connection_start_domain, so
I'll change both to the style you describe.
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 libvirt-glib 1/5] Add support for writing to streams
On Tue, Nov 22, 2011 at 12:39:28PM +, Daniel P. Berrange wrote: > + > +GType_gvir_output_stream_get_type (void) > G_GNUC_CONST; > +GVirOutputStream * _gvir_output_stream_new > (GVirStream *stream); This brings the question of how to mark functions that are internal to the library. Marc-André is in favour of using G_GNUC_INTERNAL in the function definition, you seem to prefer a _ prefix. I like the latter best, but I'd be fine with either. Christophe pgpsRnnr3wkup.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-glib 3/5] Uncomment & fix code for returning config objects
On Tue, Nov 22, 2011 at 02:19:44PM +0100, Christophe Fergeau wrote: > Hi, > > On Tue, Nov 22, 2011 at 12:39:30PM +, Daniel P. Berrange wrote: > > From: "Daniel P. Berrange" > > > > --- > > libvirt-gobject/libvirt-gobject-domain-snapshot.c |5 + > > libvirt-gobject/libvirt-gobject-interface.c |5 + > > libvirt-gobject/libvirt-gobject-network-filter.c |5 + > > libvirt-gobject/libvirt-gobject-network.c |5 + > > libvirt-gobject/libvirt-gobject-node-device.c |6 +- > > libvirt-gobject/libvirt-gobject-secret.c |6 +- > > libvirt-gobject/libvirt-gobject-storage-pool.c|5 + > > libvirt-gobject/libvirt-gobject-storage-vol.c |5 + > > 8 files changed, 8 insertions(+), 34 deletions(-) > > > > diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c > > b/libvirt-gobject/libvirt-gobject-domain-snapshot.c > > index e536d72..c65a183 100644 > > --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c > > +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c > > @@ -210,11 +210,8 @@ GVirConfigDomainSnapshot > > *gvir_domain_snapshot_get_config > > return NULL; > > } > > > > -#if 0 > > -GVirConfigDomainSnapshot *conf = gvir_config_domain_snapshot_new(xml); > > +GVirConfigDomainSnapshot *conf = > > gvir_config_domain_snapshot_new_from_xml(xml, err); > > > > g_free(xml); > > return conf; > > The xml data comes from libvirt, so it should be freed with free(), not > g_free. The same comment applies to all the other hunks. Good point, I'll fix that. 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] [libvirt-glib] Release 0.0.2
On Tue, Nov 22, 2011 at 04:27:17AM +0200, Zeeshan Ali (Khattak) wrote: > From: "Zeeshan Ali (Khattak)" > > --- > NEWS | 20 > configure.ac |3 +-- > 2 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index a0cbb60..b7b11a9 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,23 @@ > +0.0.2 - Nov 22, 2011 > + > + > +- Add API to redefine an existing domain. > +- Expicitely call virInitialize() to avoid connection races. > +- Adjust example to latest pygobject-3.0. > +- Add missing deps on libxml2-devel & libtool. > +- Don't introspect on RHEL6. > +- Add download / mailing list links to README. > +- Fix parallel build with vala bindings. > +- Fix a typo in a pkg-config file. > + > +All contributors to this release: > + > +Daniel P. Berrange > +Guido Günther > +Marc-André Lureau > +Nirbheek Chauhan > +Zeeshan Ali (Khattak) Can leave out the contributor list, since this info is included in the automatically generated ChangeLog file and AUTHORS file. ACK to the rest 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 libvirt-glib 1/5] Add support for writing to streams
On Tue, Nov 22, 2011 at 02:50:26PM +0100, Christophe Fergeau wrote: > On Tue, Nov 22, 2011 at 12:39:28PM +, Daniel P. Berrange wrote: > > + > > +GType_gvir_output_stream_get_type (void) > > G_GNUC_CONST; > > +GVirOutputStream * _gvir_output_stream_new > > (GVirStream *stream); > > This brings the question of how to mark functions that are internal to the > library. Marc-André is in favour of using G_GNUC_INTERNAL in the function > definition, you seem to prefer a _ prefix. I like the latter best, but I'd > be fine with either. On the contrary. I would gladly get rid of the '_' prefix. I only have it because the existing gvir_input_stream* functions have it, and when I tried to remove it, then GTK-DOC tried to generate pages for this type. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-glib] Don't reference GError **err parameter if it is NULL
From: "Daniel P. Berrange"
---
libvirt-gobject/libvirt-gobject-connection.c | 104 +-
libvirt-gobject/libvirt-gobject-domain.c | 70 --
libvirt-gobject/libvirt-gobject-interface.c |7 +-
libvirt-gobject/libvirt-gobject-network-filter.c |7 +-
libvirt-gobject/libvirt-gobject-network.c|7 +-
libvirt-gobject/libvirt-gobject-node-device.c|7 +-
libvirt-gobject/libvirt-gobject-secret.c |7 +-
libvirt-gobject/libvirt-gobject-storage-pool.c | 53 +++-
libvirt-gobject/libvirt-gobject-storage-vol.c|7 +-
9 files changed, 158 insertions(+), 111 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-connection.c
b/libvirt-gobject/libvirt-gobject-connection.c
index affb496..35be5e3 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -389,19 +389,21 @@ gboolean gvir_connection_open(GVirConnection *conn,
g_mutex_lock(priv->lock);
if (priv->conn) {
-*err = g_error_new(GVIR_CONNECTION_ERROR,
- 0,
- "Connection %s is already open",
- priv->uri);
+if (err)
+*err = g_error_new(GVIR_CONNECTION_ERROR,
+ 0,
+ "Connection %s is already open",
+ priv->uri);
g_mutex_unlock(priv->lock);
return FALSE;
}
if (!(priv->conn = virConnectOpen(priv->uri))) {
-*err = gvir_error_new(GVIR_CONNECTION_ERROR,
- 0,
- "Unable to open %s",
- priv->uri);
+if (err)
+*err = gvir_error_new(GVIR_CONNECTION_ERROR,
+ 0,
+ "Unable to open %s",
+ priv->uri);
g_mutex_unlock(priv->lock);
return FALSE;
}
@@ -540,9 +542,10 @@ static gchar ** fetch_list(virConnectPtr vconn,
gint i;
if ((n = count_func(vconn)) < 0) {
-*err = gvir_error_new(GVIR_CONNECTION_ERROR,
- 0,
- "Unable to count %s", name);
+if (err)
+*err = gvir_error_new(GVIR_CONNECTION_ERROR,
+ 0,
+ "Unable to count %s", name);
goto error;
}
@@ -552,9 +555,10 @@ static gchar ** fetch_list(virConnectPtr vconn,
lst = g_new(gchar *, n);
if ((n = list_func(vconn, lst, n)) < 0) {
-*err = gvir_error_new(GVIR_CONNECTION_ERROR,
- 0,
- "Unable to list %s %d", name, n);
+if (err)
+*err = gvir_error_new(GVIR_CONNECTION_ERROR,
+ 0,
+ "Unable to list %s %d", name, n);
goto error;
}
}
@@ -587,12 +591,14 @@ gboolean gvir_connection_fetch_domains(GVirConnection
*conn,
gboolean ret = FALSE;
gint i;
virConnectPtr vconn = NULL;
+GError *lerr = NULL;
g_mutex_lock(priv->lock);
if (!priv->conn) {
-*err = g_error_new(GVIR_CONNECTION_ERROR,
- 0,
- "Connection is not open");
+if (err)
+*err = g_error_new(GVIR_CONNECTION_ERROR,
+ 0,
+ "Connection is not open");
g_mutex_unlock(priv->lock);
goto cleanup;
}
@@ -605,9 +611,10 @@ gboolean gvir_connection_fetch_domains(GVirConnection
*conn,
goto cleanup;
if ((nactive = virConnectNumOfDomains(vconn)) < 0) {
-*err = gvir_error_new(GVIR_CONNECTION_ERROR,
- 0,
- "Unable to count domains");
+if (err)
+*err = gvir_error_new(GVIR_CONNECTION_ERROR,
+ 0,
+ "Unable to count domains");
goto cleanup;
}
if (nactive) {
@@ -616,9 +623,10 @@ gboolean gvir_connection_fetch_domains(GVirConnection
*conn,
active = g_new(gint, nactive);
if ((nactive = virConnectListDomains(vconn, active, nactive)) < 0) {
-*err = gvir_error_new(GVIR_CONNECTION_ERROR,
- 0,
- "Unable to list domains");
+if (err)
+*err = gvir_error_new(GVIR_CONNECTION_ERROR,
+ 0,
+ "Unable to list domains");
goto cleanup;
}
}
@@ -632,9 +640,12 @@ gboolean gvir_connection_fetch_domains(GVirConnection
*conn,
virConnectListDefinedDomains,
cancellable,
Re: [libvirt] [PATCH V1 4/9] Add a mac chain
On 11/21/2011 05:13 PM, Eric Blake wrote:
On 10/26/2011 09:12 AM, Stefan Berger wrote:
With hunks borrowed from one of David Steven's previous patches, we now
add the capability of having a 'mac' chain which is useful to filter
for multiple valid MAC addresses.
Signed-off-by: David L Stevens
Signed-off-by: Stefan Berger
+char protostr[16] = { 0, };
A bit oversized...
PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
PRINT_CHAIN(chain, chainPrefix, ifname,
(filtername) ? filtername : l3_protocols[protoidx].val);
+switch (protoidx) {
+case L2_PROTO_MAC_IDX:
+break;
+default:
+snprintf(protostr, sizeof(protostr), "-p 0x%04x ",
for a max of 11 bytes (including trailing NUL) ever printed into it, but
not the end of the world. And since you didn't check snprintf results
for failure, if we ever change the size of protostr or the length of
what we print into it, it's a slight maintenance risk we are taking on,
compared to dynamic allocation that always gets the right length. But I
don't know if it is worth replacing this snprintf with
virAsprintf/VIR_FREE overhead, so I can live with it as is.
I converted it now to virAsprintf / VIR_FREE.
@@ -2918,7 +2930,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
CMD_EXEC
"%s"
- CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
+ CMD_DEF("%s -t %s -%%c %s %%s %s -j %s")
This results in output with a double space, either:
...%s -j ...
or
...%s -p 0x -j ...
Also not the end of the world, but you may want to remove the extra
space before the -j.
Fixed. Will repost as v2.
ACK. There is some conflict resolution needed in nwfilter.rng, but that
should be trivial.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-glib] Don't reference GError **err parameter if it is NULL
On Tue, Nov 22, 2011 at 4:18 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> ---
> libvirt-gobject/libvirt-gobject-connection.c | 104
> +-
> libvirt-gobject/libvirt-gobject-domain.c | 70 --
> libvirt-gobject/libvirt-gobject-interface.c | 7 +-
> libvirt-gobject/libvirt-gobject-network-filter.c | 7 +-
> libvirt-gobject/libvirt-gobject-network.c | 7 +-
> libvirt-gobject/libvirt-gobject-node-device.c | 7 +-
> libvirt-gobject/libvirt-gobject-secret.c | 7 +-
> libvirt-gobject/libvirt-gobject-storage-pool.c | 53 +++-
> libvirt-gobject/libvirt-gobject-storage-vol.c | 7 +-
> 9 files changed, 158 insertions(+), 111 deletions(-)
>
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c
> b/libvirt-gobject/libvirt-gobject-connection.c
> index affb496..35be5e3 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -389,19 +389,21 @@ gboolean gvir_connection_open(GVirConnection *conn,
>
> g_mutex_lock(priv->lock);
> if (priv->conn) {
> - *err = g_error_new(GVIR_CONNECTION_ERROR,
> - 0,
> - "Connection %s is already open",
> - priv->uri);
> + if (err)
> + *err = g_error_new(GVIR_CONNECTION_ERROR,
> + 0,
> + "Connection %s is already open",
> + priv->uri);
> g_mutex_unlock(priv->lock);
> return FALSE;
> }
I think we better just make use of g_set_error as it does the null
check for us.
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-glib] Don't reference GError **err parameter if it is NULL
On Tue, Nov 22, 2011 at 02:18:15PM +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> ---
> libvirt-gobject/libvirt-gobject-connection.c | 104
> +-
> libvirt-gobject/libvirt-gobject-domain.c | 70 --
> libvirt-gobject/libvirt-gobject-interface.c |7 +-
> libvirt-gobject/libvirt-gobject-network-filter.c |7 +-
> libvirt-gobject/libvirt-gobject-network.c|7 +-
> libvirt-gobject/libvirt-gobject-node-device.c|7 +-
> libvirt-gobject/libvirt-gobject-secret.c |7 +-
> libvirt-gobject/libvirt-gobject-storage-pool.c | 53 +++-
> libvirt-gobject/libvirt-gobject-storage-vol.c|7 +-
> 9 files changed, 158 insertions(+), 111 deletions(-)
>
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c
> b/libvirt-gobject/libvirt-gobject-connection.c
> index affb496..35be5e3 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -389,19 +389,21 @@ gboolean gvir_connection_open(GVirConnection *conn,
>
> g_mutex_lock(priv->lock);
> if (priv->conn) {
> -*err = g_error_new(GVIR_CONNECTION_ERROR,
> - 0,
> - "Connection %s is already open",
> - priv->uri);
> +if (err)
> +*err = g_error_new(GVIR_CONNECTION_ERROR,
> + 0,
> + "Connection %s is already open",
> + priv->uri);
g_set_error(err, GVIR_CONNECTION_ERROR, 0, "Connection...); does exactly
that (test for non-NULL + g_error_new). I'm fine with this patch (ie ACK
from me), but it would be even better to introduce gvir_set_error(...) and
to use that. This can be done later though.
Christophe
> g_mutex_unlock(priv->lock);
> return FALSE;
> }
>
> if (!(priv->conn = virConnectOpen(priv->uri))) {
> -*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> - 0,
> - "Unable to open %s",
> - priv->uri);
> +if (err)
> +*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> + 0,
> + "Unable to open %s",
> + priv->uri);
> g_mutex_unlock(priv->lock);
> return FALSE;
> }
> @@ -540,9 +542,10 @@ static gchar ** fetch_list(virConnectPtr vconn,
> gint i;
>
> if ((n = count_func(vconn)) < 0) {
> -*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> - 0,
> - "Unable to count %s", name);
> +if (err)
> +*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> + 0,
> + "Unable to count %s", name);
> goto error;
> }
>
> @@ -552,9 +555,10 @@ static gchar ** fetch_list(virConnectPtr vconn,
>
> lst = g_new(gchar *, n);
> if ((n = list_func(vconn, lst, n)) < 0) {
> -*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> - 0,
> - "Unable to list %s %d", name, n);
> +if (err)
> +*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> + 0,
> + "Unable to list %s %d", name, n);
> goto error;
> }
> }
> @@ -587,12 +591,14 @@ gboolean gvir_connection_fetch_domains(GVirConnection
> *conn,
> gboolean ret = FALSE;
> gint i;
> virConnectPtr vconn = NULL;
> +GError *lerr = NULL;
>
> g_mutex_lock(priv->lock);
> if (!priv->conn) {
> -*err = g_error_new(GVIR_CONNECTION_ERROR,
> - 0,
> - "Connection is not open");
> +if (err)
> +*err = g_error_new(GVIR_CONNECTION_ERROR,
> + 0,
> + "Connection is not open");
> g_mutex_unlock(priv->lock);
> goto cleanup;
> }
> @@ -605,9 +611,10 @@ gboolean gvir_connection_fetch_domains(GVirConnection
> *conn,
> goto cleanup;
>
> if ((nactive = virConnectNumOfDomains(vconn)) < 0) {
> -*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> - 0,
> - "Unable to count domains");
> +if (err)
> +*err = gvir_error_new(GVIR_CONNECTION_ERROR,
> + 0,
> + "Unable to count domains");
> goto cleanup;
> }
> if (nactive) {
> @@ -616,9 +623,10 @@ gboolean gvir_connection_fetch_domains(GVirConnection
> *conn,
>
> active = g_new(gint, nactive);
> if ((nactive = virConnectListDomains(vconn, active, nactive))
Re: [libvirt] [PATCH V1 5/9] Add support for STP filtering
On 11/21/2011 05:50 PM, Eric Blake wrote:
On 10/26/2011 09:12 AM, Stefan Berger wrote:
This patch adds support for filtering of STP (spanning tree protocol) traffic
to the parser and makes us of the ebtables support for STP filtering. This code
now enables the filtering of traffic in chains with prefix 'stp'.
Signed-off-by: Stefan Berger
---
docs/schemas/nwfilter.rng | 154 +
src/conf/nwfilter_conf.c | 178
++
src/conf/nwfilter_conf.h | 41 ++
src/libvirt_private.syms |1
src/nwfilter/nwfilter_ebiptables_driver.c | 90 +++
5 files changed, 461 insertions(+), 3 deletions(-)
Some conflict resolution, again in the rng, and also in context for
nwfilter_conf.c, but should be trivial.
You already pre-emptively mentioned STP chains in an earlier patch, and
I see the title of 7/9 mentions more about STP documentation, so I'll
assume that between those two, the new .rng additions are properly
documented.
@@ -1047,6 +1049,136 @@ static const virXMLAttr2Struct vlanAttri
}
};
+static const virXMLAttr2Struct stpAttributes[] = {
+/* spanning tree uses a special destination MAC address */
+{
+.name = SRCMACADDR,
+.datatype = DATATYPE_MACADDR,
+.dataIdx = offsetof(virNWFilterRuleDef,
+p.stpHdrFilter.ethHdr.dataSrcMACAddr),
+},
+{
+.name = "forward-delay-hi",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataFwdDelayHi),
+},
+COMMENT_PROP(stpHdrFilter),
I'm assuming this is an accurate layout mapping the on-the-wire struct
to named fields for reference in XML attributes, although I didn't
actually go hunt down an RFC to verify. Perhaps a comment pointing tot
the STP RFC might prove handy.
I don't think it's an RFC, but an IEEE standard. I couldn't find a
better page than this one, though:
http://www.javvin.com/protocolSTP.html
@@ -2979,6 +3149,14 @@ virNWFilterRuleDefDetailsFormat(virBuffe
item->u.u16);
break;
+ case DATATYPE_UINT32_HEX:
+ asHex = true;
+ /* fallthrough */
+ case DATATYPE_UINT32:
+ virBufferAsprintf(buf, asHex ? "0x%x" : "%d",
+ item->u.u32);
%u, not %d. Otherwise you introduce a spurious negative sign on values
with the most-significant-bit set.
Fixed.
Also, I'm not entirely sure whether %u and uint32_t always match, or if
there are some 32-bit platforms where uint32_t is long and this would
trigger a type mismatch warning from gcc. On the other hand, this code
only compiles on Linux where we know uint32_t is always int; using
for PRIu32 would be more portable, but that's a separate
cleanup.
@@ -290,6 +292,16 @@ _printDataType(virNWFilterVarCombIterPtr
}
break;
+case DATATYPE_UINT32:
+case DATATYPE_UINT32_HEX:
+if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d",
+ item->u.u32)>= bufsize) {
+virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Buffer too small for uint32 type"));
Again, %u, not %d.
Fixed.
Also, this code tends to be called with a hard-coded allocation of
'number[20];', which is sufficient for uint32_t, but not long enough if
we ever expand to DATATYPE_UINT64. I'm wondering if we should use
"intprops.h" from gnulib, for the INT_BUFSIZE_BOUND() macro, rather than
a hard-coded 20. But at least this snprintf usage checked for error (I
noticed in 4/9 that you used snprintf without error checking).
Using that now with all occurrences of number[].
+return 1;
Looks like this is code addition to an existing function with positive 1
return convention, so you can defer changing it to -1 until a later
patch that cleans up the entire function (I'm only worried about
completely new functions introduced by this patch).
+case VIR_NWFILTER_RULE_PROTOCOL_STP:
+
+/* cannot handle inout direction with srcmask set in reverse dir.
+ since this clashes with -d below... */
+if (reverse&&
+HAS_ENTRY_ITEM(&rule->p.stpHdrFilter.ethHdr.dataSrcMACAddr)) {
+virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("STP filtering in %s direction with "
+ "source MAC address set is not supported"),
+ virNWFilterRuleDirectionTypeToString(
+ VIR_NWFILTER_RULE_DIRECTION_INOUT));
+return -1;
+}
+
+virBufferAsprintf(&buf,
+ CMD_DEF_PRE "%s -t %s -%%c %s %%s",
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
Looks like you'
[libvirt] [PATCH 0/4] Small fixes to non-blocking I/O in client
I missed these when reviewing the series... Jiri Denemark (4): rpc: Pass the buck only to the first available thread rpc: Fix a typo in virNetClientSendNonBlock documentation rpc: Fix handling of non-blocking calls that could not be sent rpc: Add some debug messages to virNetClient src/rpc/virnetclient.c | 15 --- 1 files changed, 12 insertions(+), 3 deletions(-) -- 1.7.8.rc3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] rpc: Fix a typo in virNetClientSendNonBlock documentation
--- src/rpc/virnetclient.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b518abd..0effceb 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1595,7 +1595,7 @@ int virNetClientSendNoReply(virNetClientPtr client, * Send a message asynchronously, without any reply * * The caller is responsible for free'ing @msg, *except* if - * this method returns -1. + * this method returns 1. * * Returns 2 on full send, 1 on partial send, 0 on no send, -1 on error */ -- 1.7.8.rc3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] rpc: Pass the buck only to the first available thread
---
src/rpc/virnetclient.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index deeeaad..b518abd 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1102,7 +1102,7 @@ static void
virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli
if (tmp != thiscall && tmp->haveThread) {
VIR_DEBUG("Passing the buck to %p", tmp);
virCondSignal(&tmp->cond);
-break;
+return;
}
tmp = tmp->next;
}
--
1.7.8.rc3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] rpc: Add some debug messages to virNetClient
---
src/rpc/virnetclient.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index c99e87c..025d270 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1040,6 +1040,7 @@ static bool
virNetClientIOEventLoopRemoveDone(virNetClientCallPtr call,
VIR_DEBUG("Waking up sleep %p", call);
virCondSignal(&call->cond);
} else {
+VIR_DEBUG("Removing completed call %p", call);
if (call->expectReply)
VIR_WARN("Got a call expecting a reply but without a waiting
thread");
ignore_value(virCondDestroy(&call->cond));
@@ -1070,6 +1071,8 @@ static bool
virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call,
if (call->haveThread) {
VIR_DEBUG("Waking up sleep %p", call);
virCondSignal(&call->cond);
+} else {
+VIR_DEBUG("Keeping unfinished call %p in the list", call);
}
return false;
} else {
@@ -1081,6 +1084,7 @@ static bool
virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call,
VIR_DEBUG("Waking up sleep %p", call);
virCondSignal(&call->cond);
} else {
+VIR_DEBUG("Removing call %p", call);
if (call->expectReply)
VIR_WARN("Got a call expecting a reply but without a waiting
thread");
ignore_value(virCondDestroy(&call->cond));
--
1.7.8.rc3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] rpc: Fix handling of non-blocking calls that could not be sent
When virNetClientIOEventLoop is called for a non-blocking call and not
even a single byte can be sent from this call without blocking, we
properly reported that to the caller which properly frees the call. But
we never removed the call from a call queue.
---
src/rpc/virnetclient.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 0effceb..c99e87c 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1256,7 +1256,12 @@ static int virNetClientIOEventLoop(virNetClientPtr
client,
/* We're not done, but we're non-blocking */
if (thiscall->nonBlock) {
virNetClientIOEventLoopPassTheBuck(client, thiscall);
-return thiscall->sentSomeData ? 1 : 0;
+if (thiscall->sentSomeData) {
+return 1;
+} else {
+virNetClientCallRemove(&client->waitDispatch, thiscall);
+return 0;
+}
}
if (fds[0].revents & (POLLHUP | POLLERR)) {
--
1.7.8.rc3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-glib] Don't reference GError **err parameter if it is NULL
On Tue, Nov 22, 2011 at 03:50:46PM +0100, Christophe Fergeau wrote:
> On Tue, Nov 22, 2011 at 02:18:15PM +, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange"
> >
> > ---
> > libvirt-gobject/libvirt-gobject-connection.c | 104
> > +-
> > libvirt-gobject/libvirt-gobject-domain.c | 70 --
> > libvirt-gobject/libvirt-gobject-interface.c |7 +-
> > libvirt-gobject/libvirt-gobject-network-filter.c |7 +-
> > libvirt-gobject/libvirt-gobject-network.c|7 +-
> > libvirt-gobject/libvirt-gobject-node-device.c|7 +-
> > libvirt-gobject/libvirt-gobject-secret.c |7 +-
> > libvirt-gobject/libvirt-gobject-storage-pool.c | 53 +++-
> > libvirt-gobject/libvirt-gobject-storage-vol.c|7 +-
> > 9 files changed, 158 insertions(+), 111 deletions(-)
> >
> > diff --git a/libvirt-gobject/libvirt-gobject-connection.c
> > b/libvirt-gobject/libvirt-gobject-connection.c
> > index affb496..35be5e3 100644
> > --- a/libvirt-gobject/libvirt-gobject-connection.c
> > +++ b/libvirt-gobject/libvirt-gobject-connection.c
> > @@ -389,19 +389,21 @@ gboolean gvir_connection_open(GVirConnection *conn,
> >
> > g_mutex_lock(priv->lock);
> > if (priv->conn) {
> > -*err = g_error_new(GVIR_CONNECTION_ERROR,
> > - 0,
> > - "Connection %s is already open",
> > - priv->uri);
> > +if (err)
> > +*err = g_error_new(GVIR_CONNECTION_ERROR,
> > + 0,
> > + "Connection %s is already open",
> > + priv->uri);
>
> g_set_error(err, GVIR_CONNECTION_ERROR, 0, "Connection...); does exactly
> that (test for non-NULL + g_error_new). I'm fine with this patch (ie ACK
> from me), but it would be even better to introduce gvir_set_error(...) and
> to use that. This can be done later though.
I'm pushing this patch now for 0.0.2 release, and then I'll followup
with a patch todo what you suggest for 0.0.3
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 V1 9/9] Improve error reporting of failures to apply filtering rules
On 11/21/2011 06:42 PM, Eric Blake wrote:
On 10/26/2011 09:12 AM, Stefan Berger wrote:
Display the executed command and failure message if a command failed to
execute.
Signed-off-by: Stefan Berger
---
src/nwfilter/nwfilter_ebiptables_driver.c | 82
++
1 file changed, 50 insertions(+), 32 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -63,10 +63,10 @@
#define CMD_DEF_PRE "cmd='"
#define CMD_DEF_POST "'"
#define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST
-#define CMD_EXEC "eval res=\\$\\(\"${cmd}\"\\)" CMD_SEPARATOR
+#define CMD_EXEC "eval res=\\$\\(\"${cmd} 2>&1 \"\\)" CMD_SEPARATOR
Okay, after turning the C literal into shell, you have:
eval res=\$\("${cmd} 2>&1 "\)
and after the shell eval, you have:
res=$(command 2>&1 )
That space after '2>&1' could be deleted, but it doesn't hurt to leave
it in.
I removed it now.
#define CMD_STOPONERR(X) \
X ? "if [ $? -ne 0 ]; then" \
-" echo \"Failure to execute command '${cmd}'.\";" \
+" echo \"Failure to execute command '${cmd}' : '${res}'.\";" \
Yes, this is a bit more informative.
" exit 1;" \
"fi" CMD_SEPARATOR \
: ""
@@ -2785,12 +2785,16 @@ err_exit:
*/
static int
ebiptablesExecCLI(virBufferPtr buf,
- int *status)
+ int *status, char **errbuf)
{
char *cmds;
char *filename;
int rc;
const char *argv[] = {NULL, NULL};
+virCommandPtr cmd;
+
+if (errbuf)
+VIR_FREE(*errbuf);
This has merge conflicts with existing patches that went in during the
meantime. I'd feel better seeing a v2 to verify proper rebasing.
ok
if (virBufferError(buf)) {
virReportOOMError();
@@ -2817,7 +2821,13 @@ ebiptablesExecCLI(virBufferPtr buf,
virMutexLock(&execCLIMutex);
-rc = virRun(argv, status);
+cmd = virCommandNewArgs(argv);
+if (errbuf)
+virCommandSetOutputBuffer(cmd, errbuf);
It looks a bit odd calling it errbuf, when it is the output collected
from stdout; perhaps calling it outbuf would make more sense.
Fixed.
@@ -3285,7 +3297,7 @@ ebtablesApplyBasicRules(const char *ifna
ebtablesLinkTmpRootChain(&buf, 1, ifname, 1);
ebtablesRenameTmpRootChain(&buf, 1, ifname);
-if (ebiptablesExecCLI(&buf,&cli_status) || cli_status != 0)
+if (ebiptablesExecCLI(&buf,&cli_status, NULL) || cli_status != 0)
You know, as long as we are cleaning things up, you could pass NULL
instead of&cli_status to enforce that the command has a 0 exit status,
so that you trim lines like this to:
if (ebiptablesExecCLI(&buf, NULL, NULL)< 0)
I replace the occurrences of the above pattern with the line you are
showing.
@@ -3874,12 +3889,13 @@ tear_down_tmpebchains:
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
}
-ebiptablesExecCLI(&buf,&cli_status);
+ebiptablesExecCLI(&buf,&cli_status, NULL);
virNWFilterReportError(VIR_ERR_BUILD_FIREWALL,
_("Some rules could not be created for "
- "interface %s."),
- ifname);
+ "interface %s : %s"),
+ ifname,
+ errmsg ? errmsg : "");
That outputs a trailing space if there was no error message. Better
might be:
virNWFilterReportError(VIR_ERR_BUILD_FIREWALL,
_("Some rules could not be created for "
"interface %s%s%s"),
ifname,
errmsg ? ": " : "",
errmsg ? errmsg : "");
Fixed.
Alas, we have an i18n nightmare. errmsg contains English text output by
the shell script, which is not marked for translation by either the
shell script (which itself is tricky - witness the libvirt-guests init
script) nor for translation in the C code that generated the shell
script. Meanwhile, since we didn't call virCommandAddEnvPassCommon() to
set the virCommand to force LC_ALL=C, that means we passed the libvirtd
setting of LC_MESSAGES on through to the child processes, such that sh
and ebtables output might also be translated. For an example, that
means someone using LC_MESSAGES=es_ES.UTF-8 could see:
Algunas reglas no han podido crearse para la interfaz eth0 : Failure to
execute command '/path/to/ebtables -t nat ...' : /bin/sh:
/path/to/ebtables: no se encontró la orden
That's a nasty mix of native/English/native output all in one very long
message. Maybe we should be rethinking the desired output format a bit,
for the sake of generating properly translated messages? Even breaking
things into multiple messages, so that each message is either translated
or English, rather than mixed, might help.
Is it worth tr
[libvirt] [PATCH v5 07/13] Add support for async close of client RPC socket
---
Notes:
Version 5:
- rebased on top of DanB's non-blocking patches; this is the only part that
required non-trivial rebase so I'm posting it for additional review
Version 4:
- no changes
Version 3:
- no changes
Version 2:
- no changes
src/rpc/virnetclient.c | 99 +++
1 files changed, 90 insertions(+), 9 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 025d270..b4b2fe7 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -101,9 +101,13 @@ struct _virNetClient {
size_t nstreams;
virNetClientStreamPtr *streams;
+
+bool wantClose;
};
+void virNetClientRequestClose(virNetClientPtr client);
+
static void virNetClientLock(virNetClientPtr client)
{
virMutexLock(&client->lock);
@@ -409,12 +413,14 @@ void virNetClientFree(virNetClientPtr client)
}
-void virNetClientClose(virNetClientPtr client)
+static void
+virNetClientCloseLocked(virNetClientPtr client)
{
-if (!client)
+VIR_DEBUG("client=%p, sock=%p", client, client->sock);
+
+if (!client->sock)
return;
-virNetClientLock(client);
virNetSocketRemoveIOCallback(client->sock);
virNetSocketFree(client->sock);
client->sock = NULL;
@@ -424,6 +430,41 @@ void virNetClientClose(virNetClientPtr client)
virNetSASLSessionFree(client->sasl);
client->sasl = NULL;
#endif
+client->wantClose = false;
+}
+
+void virNetClientClose(virNetClientPtr client)
+{
+if (!client)
+return;
+
+virNetClientLock(client);
+virNetClientCloseLocked(client);
+virNetClientUnlock(client);
+}
+
+void
+virNetClientRequestClose(virNetClientPtr client)
+{
+VIR_DEBUG("client=%p", client);
+
+virNetClientLock(client);
+
+/* If there is a thread polling for data on the socket, set wantClose flag
+ * and wake the thread up or just immediately close the socket when no-one
+ * is polling on it.
+ */
+if (client->waitDispatch) {
+char ignore = 1;
+int len = sizeof(ignore);
+
+client->wantClose = true;
+if (safewrite(client->wakeupSendFD, &ignore, len) != len)
+VIR_ERROR(_("failed to wake up polling thread"));
+} else {
+virNetClientCloseLocked(client);
+}
+
virNetClientUnlock(client);
}
@@ -1096,6 +1137,26 @@ static bool
virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call,
}
+static void
+virNetClientIOEventLoopRemoveAll(virNetClientPtr client,
+ virNetClientCallPtr thiscall)
+{
+if (!client->waitDispatch)
+return;
+
+if (client->waitDispatch == thiscall) {
+/* just pretend nothing was sent and the caller will free the call */
+thiscall->sentSomeData = false;
+} else {
+virNetClientCallPtr call = client->waitDispatch;
+virNetClientCallRemove(&client->waitDispatch, call);
+ignore_value(virCondDestroy(&call->cond));
+VIR_FREE(call->msg);
+VIR_FREE(call);
+}
+}
+
+
static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client,
virNetClientCallPtr thiscall)
{
VIR_DEBUG("Giving up the buck %p", thiscall);
@@ -1110,7 +1171,12 @@ static void
virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli
}
tmp = tmp->next;
}
+
VIR_DEBUG("No thread to pass the buck to");
+if (client->wantClose) {
+virNetClientCloseLocked(client);
+virNetClientIOEventLoopRemoveAll(client, thiscall);
+}
}
@@ -1141,11 +1207,12 @@ static int virNetClientIOEventLoop(virNetClientPtr
client,
sigset_t oldmask, blockedsigs;
int timeout = -1;
-/* If we have existing SASL decoded data we
- * don't want to sleep in the poll(), just
- * check if any other FDs are also ready
+/* If we have existing SASL decoded data we don't want to sleep in
+ * the poll(), just check if any other FDs are also ready.
+ * If the connection is going to be closed, we don't want to sleep in
+ * poll() either.
*/
-if (virNetSocketHasCachedData(client->sock))
+if (virNetSocketHasCachedData(client->sock) || client->wantClose)
timeout = 0;
/* If there are any non-blocking calls in the queue,
@@ -1208,6 +1275,11 @@ static int virNetClientIOEventLoop(virNetClientPtr
client,
fds[0].revents |= POLLIN;
}
+/* If wantClose flag is set, pretend there was an error on the socket
+ */
+if (client->wantClose)
+fds[0].revents = POLLERR;
+
if (fds[1].revents) {
VIR_DEBUG("Woken up from poll by other thread");
if (saferead(client->wakeupReadFD, &ignore, sizeof(ignore)) !=
sizeof(ignore)) {
@@ -1441,7 +1513,8 @@ static int virNetClientIO(virNetClientPtr client,
virResetLastError();
rv = virNetClientI
[libvirt] ANNOUNCE: libvirt-glib release 0.0.2
I am pleased to announce that a new release of the libvirt-glib package, version 0.0.2 is now available from ftp://libvirt.org/libvirt/glib/ The packages are GPG signed with Key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF (4096R) New in this release: - Add API to redefine an existing domain. - Explicitly call virInitialize() to avoid connection races. - Adjust example to latest pygobject-3.0. - Add missing deps on libxml2-devel & libtool. - Add support for writing to streams - Add API for creating transient domains - Change all flags parameters to be guint - Uncomment & fix code for returning object config - Ensure pools & domains hashes are non-NULL to avoid SEGV - Don't de-reference GError instances which are NULL - Update COPYING file to have latest FSF address - Update RPM specfile to include Fedora review feedback libvirt-glib comprises three distinct libraries: - libvirt-glib- Integrate with the GLib event loop and error handling - libvirt-gconfig - Representation of libvirt XML documents as GObjects - libvirt-gobject - Mapping of libvirt APIs into the GObject type system NB: While libvirt aims to be API/ABI stable, for the first few releases, we are *NOT* guaranteeing that libvirt-glib libraries are API/ABI stable. ABI stability will only be guaranteed once the bulk of the APIs have been fleshed out and proved in non-trivial application usage. We anticipate this will be within the next 6 months in order to line up with Fedora 17. Follow up comments about libvirt-glib should be directed to the regular libvir-list@redhat.com development list. Thanks to all the people involved in contributing to this release. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 0/5] NWFilter: Filter more protocols and other extensions
This patch series adds: - support for a 'mac' chain - filtering support for STP (spanning tree protocol) - better error reporting if ebtables/ip(6)table commands fail v2: - shortened series since VLAN patches have been pushed - addressed Eric Blake's comments Regards, Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 1/5] Add a mac chain
With hunks borrowed from one of David Steven's previous patches, we now
add the capability of having a 'mac' chain which is useful to filter
for multiple valid MAC addresses.
Signed-off-by: David L Stevens
Signed-off-by: Stefan Berger
---
docs/schemas/nwfilter.rng |3 +++
src/conf/nwfilter_conf.c |2 ++
src/conf/nwfilter_conf.h |2 ++
src/nwfilter/nwfilter_ebiptables_driver.c | 22 --
4 files changed, 27 insertions(+), 2 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -188,6 +188,7 @@ enum l3_proto_idx {
L3_PROTO_IPV6_IDX,
L3_PROTO_ARP_IDX,
L3_PROTO_RARP_IDX,
+L2_PROTO_MAC_IDX,
L2_PROTO_VLAN_IDX,
L3_PROTO_LAST_IDX
};
@@ -205,6 +206,7 @@ static const struct ushort_map l3_protoc
USHORTMAP_ENTRY_IDX(L3_PROTO_ARP_IDX , ETHERTYPE_ARP , "arp"),
USHORTMAP_ENTRY_IDX(L3_PROTO_RARP_IDX, ETHERTYPE_REVARP, "rarp"),
USHORTMAP_ENTRY_IDX(L2_PROTO_VLAN_IDX, ETHERTYPE_VLAN , "vlan"),
+USHORTMAP_ENTRY_IDX(L2_PROTO_MAC_IDX, 0 , "mac"),
USHORTMAP_ENTRY_IDX(L3_PROTO_LAST_IDX, 0 , NULL),
};
@@ -2806,11 +2808,25 @@ ebtablesCreateTmpSubChain(ebiptablesRule
char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
: CHAINPREFIX_HOST_OUT_TEMP;
+char *protostr = NULL;
PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
PRINT_CHAIN(chain, chainPrefix, ifname,
(filtername) ? filtername : l3_protocols[protoidx].val);
+switch (protoidx) {
+case L2_PROTO_MAC_IDX:
+break;
+default:
+virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr);
+break;
+}
+
+if (!protostr) {
+virReportOOMError();
+return -1;
+}
+
virBufferAsprintf(&buf,
CMD_DEF("%s -t %s -F %s") CMD_SEPARATOR
CMD_EXEC
@@ -2819,7 +2835,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
CMD_EXEC
"%s"
- CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
+ CMD_DEF("%s -t %s -%%c %s %%s %s -j %s")
CMD_SEPARATOR
CMD_EXEC
"%s",
@@ -2831,10 +2847,12 @@ ebtablesCreateTmpSubChain(ebiptablesRule
CMD_STOPONERR(stopOnError),
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
- rootchain, l3_protocols[protoidx].attr, chain,
+ rootchain, protostr, chain,
CMD_STOPONERR(stopOnError));
+VIR_FREE(protostr);
+
if (virBufferError(&buf) ||
VIR_EXPAND_N(tmp, count, 1) < 0) {
virReportOOMError();
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -82,6 +82,7 @@ VIR_ENUM_IMPL(virNWFilterEbtablesTable,
VIR_ENUM_IMPL(virNWFilterChainSuffix, VIR_NWFILTER_CHAINSUFFIX_LAST,
"root",
+ "mac",
"vlan",
"arp",
"rarp",
@@ -128,6 +129,7 @@ struct int_map {
static const struct int_map chain_priorities[] = {
INTMAP_ENTRY(NWFILTER_ROOT_FILTER_PRI, "root"),
+INTMAP_ENTRY(NWFILTER_MAC_FILTER_PRI, "mac"),
INTMAP_ENTRY(NWFILTER_VLAN_FILTER_PRI, "vlan"),
INTMAP_ENTRY(NWFILTER_IPV4_FILTER_PRI, "ipv4"),
INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, "ipv6"),
Index: libvirt-acl/src/conf/nwfilter_conf.h
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -378,6 +378,7 @@ enum virNWFilterEbtablesTableType {
# define NWFILTER_MAX_FILTER_PRIORITY MAX_RULE_PRIORITY
# define NWFILTER_ROOT_FILTER_PRI 0
+# define NWFILTER_MAC_FILTER_PRI -800
# define NWFILTER_VLAN_FILTER_PRI -750
# define NWFILTER_IPV4_FILTER_PRI -700
# define NWFILTER_IPV6_FILTER_PRI -600
@@ -456,6 +457,7 @@ struct _virNWFilterEntry {
enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_ROOT = 0,
+VIR_NWFILTER_CHAINSUFFIX_MAC,
VIR_NWFILTER_CHAINSUFFIX_VLAN,
VIR_NWFILTER_CHAINSUFFIX_ARP,
VIR_NWFILTER_CHAINSUFFIX_RARP,
Index: libvirt-acl/docs/schemas/nwfilter.rng
===
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -297,6 +297,9 @@
root
+
[libvirt] [PATCH V2 3/5] Add test cases for STP traffic filtering
This patch adds a few test cases for the XML parsing of STP filtering nodes.
Signed-off-by: Stefan Berger
---
tests/nwfilterxml2xmlin/stp-test.xml | 26 ++
tests/nwfilterxml2xmlout/stp-test.xml | 12
tests/nwfilterxml2xmltest.c |1 +
3 files changed, 39 insertions(+)
Index: libvirt-acl/tests/nwfilterxml2xmlin/stp-test.xml
===
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlin/stp-test.xml
@@ -0,0 +1,26 @@
+
+ 5c6d49af-b071-6127-b4ec-6f8ed4b55335
+
+
+
+
+
+
+
+
+
+
+
+
+
Index: libvirt-acl/tests/nwfilterxml2xmltest.c
===
--- libvirt-acl.orig/tests/nwfilterxml2xmltest.c
+++ libvirt-acl/tests/nwfilterxml2xmltest.c
@@ -109,6 +109,7 @@ mymain(void)
DO_TEST("mac-test", true);
DO_TEST("vlan-test", true);
+DO_TEST("stp-test", false);
DO_TEST("arp-test", true);
DO_TEST("rarp-test", true);
DO_TEST("ip-test", true);
Index: libvirt-acl/tests/nwfilterxml2xmlout/stp-test.xml
===
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlout/stp-test.xml
@@ -0,0 +1,12 @@
+
+ 5c6d49af-b071-6127-b4ec-6f8ed4b55335
+
+
+
+
+
+
+
+
+
+
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 4/5] Add documentation for STP filtering support
Add documentation for the STP filtering support. Describe the XML attributes that are supported. Signed-off-by: Stefan Berger --- docs/formatnwfilter.html.in | 142 1 file changed, 142 insertions(+) Index: libvirt-acl/docs/formatnwfilter.html.in === --- libvirt-acl.orig/docs/formatnwfilter.html.in +++ libvirt-acl/docs/formatnwfilter.html.in @@ -603,6 +603,148 @@ Valid Strings for encap-protocol are: arp, ipv4, ipv6 +STP (Spanning Tree Protocol) + (Since 0.9.8) + + + Protocol ID: stp + + Note: Rules of this type should go either into the root or + stp chain. + + + + Attribute + Datatype + Semantics + + + srcmacaddr + MAC_ADDR + MAC address of sender + + + srcmacmask + MAC_MASK + Mask applied to MAC address of sender + + + type + UINT8 + Bridge Protcol Data Unit (BPDU) type + + + flags + UINT8 + BPDU flag + + + root-priority + UINT16 + Root priority (range start) + + + root-priority-hi + UINT16 + Root priority range end + + + root-address + MAC_ADDRESS + Root MAC address + + + root-address-mask + MAC_MASK + Root MAC address mask + + + root-cost + UINT32 + Root path cost (range start) + + + root-cost-hi + UINT32 + Root path cost range end + + + sender-priority + UINT16 + Sender priority (range start) + + + sender-priority-hi + UINT16 + Sender priority range end + + + sender-address + MAC_ADDRESS + BPDU sender MAC address + + + sender-address-mask + MAC_MASK + BPDU sender MAC address mask + + + port + UINT16 + Port identifier (range start) + + + port_hi + UINT16 + Port identifier range end + + + msg-age + UINT16 + Message age timer (range start) + + + msg-age-hi + UINT16 + Message age timer range end + + + max-age + UINT16 + Maximum age timer (range start) + + + max-age-hi + UINT16 + Maximum age timer range end + + + hello-time + UINT16 + Hello time timer (range start) + + + hello-time-hi + UINT16 + Hello time timer range end + + + forward-delay + UINT16 + Forward delay (range start) + + + forward-delay-hi + UINT16 + Forward delay range end + + + comment + STRING + text with max. 256 characters + + + ARP/RARP Protocol ID: arp or rarp -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 5/5] Improve error reporting of failures to apply filtering rules
Display the executed command and failure message if a command failed to
execute.
Signed-off-by: Stefan Berger
---
v2:
- addressing Eric Blake's comments
---
src/nwfilter/nwfilter_ebiptables_driver.c | 86 +-
1 file changed, 50 insertions(+), 36 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -65,10 +65,10 @@
#define CMD_DEF_PRE "cmd='"
#define CMD_DEF_POST "'"
#define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST
-#define CMD_EXEC "eval res=\\$\\(\"${cmd}\"\\)" CMD_SEPARATOR
+#define CMD_EXEC "eval res=\\$\\(\"${cmd} 2>&1\"\\)" CMD_SEPARATOR
#define CMD_STOPONERR(X) \
X ? "if [ $? -ne 0 ]; then" \
-" echo \"Failure to execute command '${cmd}'.\";" \
+" echo \"Failure to execute command '${cmd}' : '${res}'.\";" \
" exit 1;" \
"fi" CMD_SEPARATOR \
: ""
@@ -2707,6 +2707,10 @@ ebiptablesDisplayRuleInstance(virConnect
*execute.
* @status: Pointer to an integer for returning the WEXITSTATUS of the
*commands executed via the script the was run.
+ * @outbuf: Optional pointer to a string that will hold the buffer with
+ * output of the executed command. The actual buffer holding
+ * the message will be newly allocated by this function and
+ * any passed in buffer freed first.
*
* Returns 0 in case of success, < 0 in case of an error. The returned
* value is NOT the result of running the commands inside the shell
@@ -2718,17 +2722,24 @@ ebiptablesDisplayRuleInstance(virConnect
*/
static int
ebiptablesExecCLI(virBufferPtr buf,
- int *status)
+ int *status, char **outbuf)
{
int rc = -1;
virCommandPtr cmd;
-*status = 0;
+if (status)
+ *status = 0;
+
if (!virBufferError(buf) && !virBufferUse(buf))
return 0;
+if (outbuf)
+VIR_FREE(*outbuf);
+
cmd = virCommandNewArgList("/bin/sh", "-c", NULL);
virCommandAddArgBuffer(cmd, buf);
+if (outbuf)
+virCommandSetOutputBuffer(cmd, outbuf);
virMutexLock(&execCLIMutex);
@@ -3138,7 +3149,6 @@ ebtablesApplyBasicRules(const char *ifna
const unsigned char *macaddr)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
-int cli_status;
char chain[MAX_CHAINNAME_LENGTH];
char chainPrefix = CHAINPREFIX_HOST_IN_TEMP;
char macaddr_str[VIR_MAC_STRING_BUFLEN];
@@ -3193,7 +3203,7 @@ ebtablesApplyBasicRules(const char *ifna
ebtablesLinkTmpRootChain(&buf, 1, ifname, 1);
ebtablesRenameTmpRootChain(&buf, 1, ifname);
-if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
goto tear_down_tmpebchains;
return 0;
@@ -3229,7 +3239,6 @@ ebtablesApplyDHCPOnlyRules(const char *i
const char *dhcpserver)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
-int cli_status;
char chain_in [MAX_CHAINNAME_LENGTH],
chain_out[MAX_CHAINNAME_LENGTH];
char macaddr_str[VIR_MAC_STRING_BUFLEN];
@@ -3309,7 +3318,7 @@ ebtablesApplyDHCPOnlyRules(const char *i
ebtablesRenameTmpRootChain(&buf, 1, ifname);
ebtablesRenameTmpRootChain(&buf, 0, ifname);
-if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
goto tear_down_tmpebchains;
VIR_FREE(srcIPParam);
@@ -3342,7 +3351,6 @@ static int
ebtablesApplyDropAllRules(const char *ifname)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
-int cli_status;
char chain_in [MAX_CHAINNAME_LENGTH],
chain_out[MAX_CHAINNAME_LENGTH];
@@ -3382,7 +3390,7 @@ ebtablesApplyDropAllRules(const char *if
ebtablesRenameTmpRootChain(&buf, 1, ifname);
ebtablesRenameTmpRootChain(&buf, 0, ifname);
-if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
goto tear_down_tmpebchains;
return 0;
@@ -3425,7 +3433,7 @@ static int ebtablesCleanAll(const char *
ebtablesRemoveTmpRootChain(&buf, 1, ifname);
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
-ebiptablesExecCLI(&buf, &cli_status);
+ebiptablesExecCLI(&buf, &cli_status, NULL);
return 0;
}
@@ -3582,6 +3590,7 @@ ebiptablesApplyNewRules(virConnectPtr co
bool haveIp6tables = false;
ebiptablesRuleInstPtr ebtChains = NULL;
int nEbtChains = 0;
+char *errmsg = NULL;
if (!chains_in_set || !chains_out_set) {
virReportOOMError();
@@ -3620,7 +3629,7 @@ ebiptablesApplyNewRules(virConnectPtr co
ebtablesRemoveTmpSubChains(&buf, ifname);
ebtablesRemoveTmpRootChain(&buf, 1, ifname);
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
-ebiptablesExecCLI
[libvirt] [PATCH V2 2/5] Add support for STP filtering
This patch adds support for filtering of STP (spanning tree protocol) traffic
to the parser and makes us of the ebtables support for STP filtering. This code
now enables the filtering of traffic in chains with prefix 'stp'.
Signed-off-by: Stefan Berger
---
v2:
- addressing Eric Blake's comments
- replacing occurrences of number[20] with number[INT_BUFFER_BOUND(uint32)];
this
also works for IP masks which are expressed as "%d".
---
docs/schemas/nwfilter.rng | 152 +
src/conf/nwfilter_conf.c | 178 ++
src/conf/nwfilter_conf.h | 41 ++
src/libvirt_private.syms |1
src/nwfilter/nwfilter_ebiptables_driver.c | 100
5 files changed, 465 insertions(+), 7 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -84,6 +84,7 @@ VIR_ENUM_IMPL(virNWFilterChainSuffix, VI
"root",
"mac",
"vlan",
+ "stp",
"arp",
"rarp",
"ipv4",
@@ -93,6 +94,7 @@ VIR_ENUM_IMPL(virNWFilterRuleProtocol, V
"none",
"mac",
"vlan",
+ "stp",
"arp",
"rarp",
"ip",
@@ -1047,6 +1049,136 @@ static const virXMLAttr2Struct vlanAttri
}
};
+static const virXMLAttr2Struct stpAttributes[] = {
+/* spanning tree uses a special destination MAC address */
+{
+.name = SRCMACADDR,
+.datatype = DATATYPE_MACADDR,
+.dataIdx = offsetof(virNWFilterRuleDef,
+p.stpHdrFilter.ethHdr.dataSrcMACAddr),
+},
+{
+.name = SRCMACMASK,
+.datatype = DATATYPE_MACMASK,
+.dataIdx = offsetof(virNWFilterRuleDef,
+p.stpHdrFilter.ethHdr.dataSrcMACMask),
+},
+{
+.name = "type",
+.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataType),
+},
+{
+.name = "flags",
+.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataFlags),
+},
+{
+.name = "root-priority",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootPri),
+},
+{
+.name = "root-priority-hi",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootPriHi),
+},
+{
+.name = "root-address",
+.datatype = DATATYPE_MACADDR,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootAddr),
+},
+{
+.name = "root-address-mask",
+.datatype = DATATYPE_MACMASK,
+.dataIdx = offsetof(virNWFilterRuleDef,
p.stpHdrFilter.dataRootAddrMask),
+},
+{
+.name = "root-cost",
+.datatype = DATATYPE_UINT32 | DATATYPE_UINT32_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootCost),
+},
+{
+.name = "root-cost-hi",
+.datatype = DATATYPE_UINT32 | DATATYPE_UINT32_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootCostHi),
+},
+{
+.name = "sender-priority",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataSndrPrio),
+},
+{
+.name = "sender-priority-hi",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataSndrPrioHi),
+},
+{
+.name = "sender-address",
+.datatype = DATATYPE_MACADDR,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataSndrAddr),
+},
+{
+.name = "sender-address-mask",
+.datatype = DATATYPE_MACMASK,
+.dataIdx = offsetof(virNWFilterRuleDef,
p.stpHdrFilter.dataSndrAddrMask),
+},
+{
+.name = "port",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataPort),
+},
+{
+.name = "port-hi",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataPortHi),
+},
+{
+.name = "age",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataAge),
+},
+{
+.name = "age-hi",
+.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataAgeHi),
+},
+{
+.name = "max-age",
+.datatype = DATATYPE
[libvirt] [PATCH v2 2/2] nwfilter: use shell variable to invoke 'ip(6)tables' command
Introduce a shell variable 'IBT' to invoke the ip(6)tables command.
Tested with libvirt-tck.
---
v2:
- rebased
---
src/nwfilter/nwfilter_ebiptables_driver.c | 313 ++
1 file changed, 155 insertions(+), 158 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -147,6 +147,10 @@ static const char ebiptables_script_set_
#define NWFILTER_SET_EBTABLES_SHELLVAR(BUFPTR) \
virBufferAsprintf(BUFPTR, "EBT=%s\n", ebtables_cmd_path);
+#define NWFILTER_SET_IPTABLES_SHELLVAR(BUFPTR) \
+virBufferAsprintf(BUFPTR, "IPT=%s\n", iptables_cmd_path);
+#define NWFILTER_SET_IP6TABLES_SHELLVAR(BUFPTR) \
+virBufferAsprintf(BUFPTR, "IPT=%s\n", ip6tables_cmd_path);
#define VIRT_IN_CHAIN "libvirt-in"
#define VIRT_OUT_CHAIN "libvirt-out"
@@ -494,66 +498,60 @@ ebtablesHandleEthHdr(virBufferPtr buf,
/ iptables support /
-static int iptablesLinkIPTablesBaseChain(const char *iptables_cmd,
- virBufferPtr buf,
+static int iptablesLinkIPTablesBaseChain(virBufferPtr buf,
const char *udchain,
const char *syschain,
unsigned int pos,
int stopOnError)
{
virBufferAsprintf(buf,
- "res=$(%s -L %s -n --line-number | "
+ "res=$($IPT -L %s -n --line-number | "
"%s \" %s \")\n"
"if [ $? -ne 0 ]; then\n"
- " %s -I %s %d -j %s\n"
+ " $IPT -I %s %d -j %s\n"
"else\n"
" r=$(echo $res | %s '{print $1}')\n"
" if [ \"${r}\" != \"%d\" ]; then\n"
- "" CMD_DEF("%s -I %s %d -j %s") CMD_SEPARATOR
+ "" CMD_DEF("$IPT -I %s %d -j %s") CMD_SEPARATOR
"" CMD_EXEC
"%s"
"r=$(( $r + 1 ))\n"
- "" CMD_DEF("%s -D %s ${r}") CMD_SEPARATOR
+ "" CMD_DEF("$IPT -D %s ${r}") CMD_SEPARATOR
"" CMD_EXEC
"%s"
" fi\n"
"fi\n",
- iptables_cmd, syschain,
+ syschain,
grep_cmd_path, udchain,
- iptables_cmd, syschain, pos, udchain,
+ syschain, pos, udchain,
gawk_cmd_path,
pos,
- iptables_cmd, syschain, pos, udchain,
+ syschain, pos, udchain,
CMD_STOPONERR(stopOnError),
- iptables_cmd, syschain,
+ syschain,
CMD_STOPONERR(stopOnError));
return 0;
}
-static int iptablesCreateBaseChains(const char *iptables_cmd,
-virBufferPtr buf)
+static int iptablesCreateBaseChains(virBufferPtr buf)
{
-virBufferAsprintf(buf,"%s -N " VIRT_IN_CHAIN CMD_SEPARATOR
- "%s -N " VIRT_OUT_CHAIN CMD_SEPARATOR
- "%s -N " VIRT_IN_POST_CHAIN CMD_SEPARATOR
- "%s -N " HOST_IN_CHAIN CMD_SEPARATOR,
- iptables_cmd,
- iptables_cmd,
- iptables_cmd,
- iptables_cmd);
-iptablesLinkIPTablesBaseChain(iptables_cmd, buf,
+virBufferAddLit(buf, "$IPT -N " VIRT_IN_CHAIN CMD_SEPARATOR
+ "$IPT -N " VIRT_OUT_CHAIN CMD_SEPARATOR
+ "$IPT -N " VIRT_IN_POST_CHAIN CMD_SEPARATOR
+ "$IPT -N " HOST_IN_CHAIN CMD_SEPARATOR);
+iptablesLinkIPTablesBaseChain(buf,
VIRT_IN_CHAIN , "FORWARD", 1, 1);
-iptablesLinkIPTablesBaseChain(iptables_cmd, buf,
+iptablesLinkIPTablesBaseChain(buf,
VIRT_OUT_CHAIN, "FORWARD", 2, 1);
-iptablesLinkIPTablesBaseChain(iptables_cmd, buf,
+iptablesLinkIPTablesBaseChain(buf,
VIRT_IN_POST_CHAIN, "FORWARD", 3, 1);
-iptablesLinkIPTablesBaseChain(iptables_cmd, buf,
+iptablesLinkIPTablesBaseChain(buf,
HOST_IN_CHAIN , "INPUT" , 1, 1);
return 0;
@@ -561,8 +559,7 @@ static int iptablesCreateBaseChains(cons
static int
-iptablesCreateTmpRootChain(const char *iptables_cmd,
- virBufferPtr buf,
+iptablesCrea
[libvirt] [PATCH v2 1/2] nwfilter: use shell variable to invoke 'ebtables' command
Introduce a shell variable 'EBT' to invoke the ebtables command.
Hard-code the used ebtables table to '-t nat'.
Tested with libvirt-tck.
---
v2:
- rebased
---
src/nwfilter/nwfilter_ebiptables_driver.c | 170 +-
1 file changed, 97 insertions(+), 73 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -47,7 +47,6 @@
#define VIR_FROM_THIS VIR_FROM_NWFILTER
-#define EBTABLES_DEFAULT_TABLE "nat"
#define EBTABLES_CHAIN_INCOMING "PREROUTING"
#define EBTABLES_CHAIN_OUTGOING "POSTROUTING"
@@ -87,7 +86,6 @@ static char *ip6tables_cmd_path;
static char *grep_cmd_path;
static char *gawk_cmd_path;
-
#define PRINT_ROOT_CHAIN(buf, prefix, ifname) \
snprintf(buf, sizeof(buf), "libvirt-%c-%s", prefix, ifname)
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
@@ -111,7 +109,7 @@ static const char ebtables_script_func_c
"collect_chains()\n"
"{\n"
" for tmp2 in $*; do\n"
-"for tmp in $(%s -t %s -L $tmp2 | \\\n"
+"for tmp in $($EBT -t nat -L $tmp2 | \\\n"
" sed -n \"/Bridge chain/,\\$ s/.*-j ([%s]-.*)/1/p\");\n"
"do\n"
" echo $tmp\n"
@@ -123,8 +121,8 @@ static const char ebtables_script_func_c
static const char ebiptables_script_func_rm_chains[] =
"rm_chains()\n"
"{\n"
-" for tmp in $*; do %s -t %s -F $tmp; done\n"
-" for tmp in $*; do %s -t %s -X $tmp; done\n"
+" for tmp in $*; do $EBT -t nat -F $tmp; done\n"
+" for tmp in $*; do $EBT -t nat -X $tmp; done\n"
"}\n";
static const char ebiptables_script_func_rename_chains[] =
@@ -132,8 +130,8 @@ static const char ebiptables_script_func
"{\n"
" for tmp in $*; do\n"
"case $tmp in\n"
-" %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n"
-" %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n"
+" %c*) $EBT -t nat -E $tmp %c${tmp#?} ;;\n"
+" %c*) $EBT -t nat -E $tmp %c${tmp#?} ;;\n"
"esac\n"
" done\n"
"}\n";
@@ -147,6 +145,9 @@ static const char ebiptables_script_set_
#define NWFILTER_FUNC_RENAME_CHAINS ebiptables_script_func_rename_chains
#define NWFILTER_FUNC_SET_IFS ebiptables_script_set_ifs
+#define NWFILTER_SET_EBTABLES_SHELLVAR(BUFPTR) \
+virBufferAsprintf(BUFPTR, "EBT=%s\n", ebtables_cmd_path);
+
#define VIRT_IN_CHAIN "libvirt-in"
#define VIRT_OUT_CHAIN "libvirt-out"
#define VIRT_IN_POST_CHAIN "libvirt-in-post"
@@ -1992,9 +1993,8 @@ ebtablesCreateRuleInstance(char chainPre
case VIR_NWFILTER_RULE_PROTOCOL_MAC:
virBufferAsprintf(&buf,
- CMD_DEF_PRE "%s -t %s -%%c %s %%s",
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
-
+ CMD_DEF_PRE "$EBT -t nat -%%c %s %%s",
+ chain);
if (ebtablesHandleEthHdr(&buf,
vars,
@@ -2017,8 +2017,8 @@ ebtablesCreateRuleInstance(char chainPre
case VIR_NWFILTER_RULE_PROTOCOL_VLAN:
virBufferAsprintf(&buf,
- CMD_DEF_PRE "%s -t %s -%%c %s %%s",
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
+ CMD_DEF_PRE "$EBT -t nat -%%c %s %%s",
+ chain);
if (ebtablesHandleEthHdr(&buf,
@@ -2084,8 +2084,8 @@ ebtablesCreateRuleInstance(char chainPre
}
virBufferAsprintf(&buf,
- CMD_DEF_PRE "%s -t %s -%%c %s %%s",
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
+ CMD_DEF_PRE "$EBT -t nat -%%c %s %%s",
+ chain);
if (ebtablesHandleEthHdr(&buf,
@@ -2122,8 +2122,8 @@ ebtablesCreateRuleInstance(char chainPre
case VIR_NWFILTER_RULE_PROTOCOL_RARP:
virBufferAsprintf(&buf,
- CMD_DEF_PRE "%s -t %s -%%c %s %%s",
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
+ CMD_DEF_PRE "$EBT -t nat -%%c %s %%s",
+ chain);
if (ebtablesHandleEthHdr(&buf,
vars,
@@ -2231,8 +2231,8 @@ ebtablesCreateRuleInstance(char chainPre
case VIR_NWFILTER_RULE_PROTOCOL_IP:
virBufferAsprintf(&buf,
- CMD_DEF_PRE "%s -t %s -%%c %s %%s",
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
+ CMD_DEF_PRE "$EBT -t nat -%%c %s %%s",
+ chain);
if (ebtablesHandleEthHdr(&buf,
vars,
@@ -2367,8 +2367,8 @@ ebtablesCreateRuleInstance(char chainPre
case VIR_NWFILTER_RULE_PROTOCOL_IPV6:
virBufferAsprintf(&buf,
-
Re: [libvirt] [PATCH V1 5/9] Add support for STP filtering
On 11/22/2011 07:51 AM, Stefan Berger wrote: > On 11/21/2011 05:50 PM, Eric Blake wrote: >> On 10/26/2011 09:12 AM, Stefan Berger wrote: >>> This patch adds support for filtering of STP (spanning tree protocol) >>> traffic >>> to the parser and makes us of the ebtables support for STP filtering. >>> This code >>> now enables the filtering of traffic in chains with prefix 'stp'. >>> >> I'm assuming this is an accurate layout mapping the on-the-wire struct >> to named fields for reference in XML attributes, although I didn't >> actually go hunt down an RFC to verify. Perhaps a comment pointing tot >> the STP RFC might prove handy. >> > I don't think it's an RFC, but an IEEE standard. I couldn't find a > better page than this one, though: > > http://www.javvin.com/protocolSTP.html Then how about this comment: /* STP is documented by IEEE 802.1D; for a synopsis, * see http://www.javvin.com/protocolSTP.html */ -- Eric Blake ebl...@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] virsh: Allow other escape characters for console
Currently virsh supports only ^] as escape character for console.
However, some users might want to use something else. This patch
creates such ability by specifying '-e' switch on virsh command
line.
---
Okay, this patch is meant as RFC mainly but if it got enough ACKs
I will not hesitate to push it. My biggest concern is the way
of telling virsh customized sequence. I am not big fan of new switch,
however we lack virsh.conf in $conf_dir. Maybe it is the right time
for creating it.
Another approach is to pass the sequence as parameter directly to
'console' command.
What's your opinion?
tools/console.c | 28 +++-
tools/console.h |4 +++-
tools/virsh.c | 20
tools/virsh.pod |5 +
4 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/tools/console.c b/tools/console.c
index 0f85bc7..db2e17e 100644
--- a/tools/console.c
+++ b/tools/console.c
@@ -43,9 +43,13 @@
# include "memory.h"
# include "virterror_internal.h"
-
-/* ie Ctrl-] as per telnet */
-# define CTRL_CLOSE_BRACKET '\35'
+/*
+ * Convert given character to control character.
+ * Basically, we take lower 5 bits unless given
+ * character is DEL (represented by '?'). Then
+ * we return 127
+ */
+# define CONTROL(c) ((c) == '?' ? ((c) | 0x40) : ((c) & 0x1f))
# define VIR_FROM_THIS VIR_FROM_NONE
@@ -66,6 +70,8 @@ struct virConsole {
struct virConsoleBuffer streamToTerminal;
struct virConsoleBuffer terminalToStream;
+
+char escapeChar;
};
static int got_signal = 0;
@@ -215,7 +221,7 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
virConsoleShutdown(con);
return;
}
-if (con->terminalToStream.data[con->terminalToStream.offset] ==
CTRL_CLOSE_BRACKET) {
+if (con->terminalToStream.data[con->terminalToStream.offset] ==
con->escapeChar) {
virConsoleShutdown(con);
return;
}
@@ -278,7 +284,18 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
}
-int vshRunConsole(virDomainPtr dom, const char *dev_name)
+static char
+vshGetEscapeChar(const char *s)
+{
+if (*s == '^')
+return CONTROL(s[1]);
+
+return *s;
+}
+
+int vshRunConsole(virDomainPtr dom,
+ const char *dev_name,
+ const char *escape_seq)
{
int ret = -1;
struct termios ttyattr, rawattr;
@@ -326,6 +343,7 @@ int vshRunConsole(virDomainPtr dom, const char *dev_name)
goto cleanup;
}
+con->escapeChar = vshGetEscapeChar(escape_seq);
con->st = virStreamNew(virDomainGetConnect(dom),
VIR_STREAM_NONBLOCK);
if (!con->st)
diff --git a/tools/console.h b/tools/console.h
index 9b05ff1..8cca08f 100644
--- a/tools/console.h
+++ b/tools/console.h
@@ -25,7 +25,9 @@
# ifndef WIN32
-int vshRunConsole(virDomainPtr dom, const char *dev_name);
+int vshRunConsole(virDomainPtr dom,
+ const char *dev_name,
+ const char *escape_seq);
# endif /* !WIN32 */
diff --git a/tools/virsh.c b/tools/virsh.c
index 89fb4e7..1b84980 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -75,6 +75,9 @@ static char *progname;
int) ((T)->tv_sec - (U)->tv_sec)) * 100.0 + \
((int) ((T)->tv_usec - (U)->tv_usec))) / 1000.0)
+/* Default escape char Ctrl-] as per telnet */
+#define CTRL_CLOSE_BRACKET "^]"
+
/**
* The log configuration
*/
@@ -249,6 +252,7 @@ typedef struct __vshControl {
virDomainGetState is not supported */
bool useSnapshotOld;/* cannot use virDomainSnapshotGetParent or
virDomainSnapshotNumChildren */
+const char *escapeChar; /* Escape character for domain console */
} __vshControl;
typedef struct vshCmdGrp {
@@ -796,8 +800,8 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char
*name)
}
vshPrintExtra(ctl, _("Connected to domain %s\n"), virDomainGetName(dom));
-vshPrintExtra(ctl, "%s", _("Escape character is ^]\n"));
-if (vshRunConsole(dom, name) == 0)
+vshPrintExtra(ctl, _("Escape character is %s\n"), ctl->escapeChar);
+if (vshRunConsole(dom, name, ctl->escapeChar) == 0)
ret = true;
cleanup:
@@ -16817,6 +16821,7 @@ vshDeinit(vshControl *ctl)
vshReadlineDeinit(ctl);
vshCloseLogFile(ctl);
VIR_FREE(ctl->name);
+VIR_FREE(ctl->escapeChar);
if (ctl->conn) {
int ret;
if ((ret = virConnectClose(ctl->conn)) != 0) {
@@ -16848,7 +16853,8 @@ vshUsage(void)
"-t | --timing print timing information\n"
"-l | --logoutput logging to file\n"
"-v | --version[=short] program version\n"
- "-V | --version=long version and full
options\n\n"
+ "-V | --version=long version and full options\n"
+ "-e | --escap
Re: [libvirt] [PATCH] virsh: Allow other escape characters for console
On 11/22/2011 09:18 AM, Michal Privoznik wrote:
> Currently virsh supports only ^] as escape character for console.
> However, some users might want to use something else. This patch
> creates such ability by specifying '-e' switch on virsh command
> line.
> ---
> Okay, this patch is meant as RFC mainly but if it got enough ACKs
> I will not hesitate to push it. My biggest concern is the way
> of telling virsh customized sequence. I am not big fan of new switch,
> however we lack virsh.conf in $conf_dir. Maybe it is the right time
> for creating it.
> Another approach is to pass the sequence as parameter directly to
> 'console' command.
I'm leaning 60-40 towards 'virsh -e', since both 'virsh console' and
'virsh start --console' would benefit from shared code, rather than
having to make both of those functions parse in an alternate escape
character.
I agree that a virsh.conf would make it nice to set preferences without
having to always spell it out on the command line, in which case having
virsh.conf serve as an alternate to 'virsh -e ... console' makes more
sense than having it serve as an alternate to 'virsh console -e ...'
(that is, it seems like having virsh.conf override defaults for global
settings makes more sense than having it override defaults of
per-command settings), so that would sway my opinion to 70-30 in favor
of a global -e.
> -
> -/* ie Ctrl-] as per telnet */
> -# define CTRL_CLOSE_BRACKET '\35'
> +/*
> + * Convert given character to control character.
> + * Basically, we take lower 5 bits unless given
> + * character is DEL (represented by '?'). Then
> + * we return 127
> + */
> +# define CONTROL(c) ((c) == '?' ? ((c) | 0x40) : ((c) & 0x1f))
I've usually seen this written:
CONTROL(c) ((c) ^ 0x40)
This is ASCII-specific, but do we care about EBCDIC?
>
> -int vshRunConsole(virDomainPtr dom, const char *dev_name)
> +static char
> +vshGetEscapeChar(const char *s)
> +{
> +if (*s == '^')
> +return CONTROL(s[1]);
> +
> +return *s;
> +}
Should we sanity check that the string s is exactly 1 or 2 bytes, that
it is not a 1-byte ^, and that if it is 2 bytes, it starts with ^?
>
> +/* Default escape char Ctrl-] as per telnet */
> +#define CTRL_CLOSE_BRACKET "^]"
> +
> /**
> * The log configuration
> */
> @@ -249,6 +252,7 @@ typedef struct __vshControl {
> virDomainGetState is not supported */
> bool useSnapshotOld;/* cannot use virDomainSnapshotGetParent or
> virDomainSnapshotNumChildren */
> +const char *escapeChar; /* Escape character for domain console */
> } __vshControl;
>
> typedef struct vshCmdGrp {
> @@ -796,8 +800,8 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const
> char *name)
> }
>
> vshPrintExtra(ctl, _("Connected to domain %s\n"), virDomainGetName(dom));
> -vshPrintExtra(ctl, "%s", _("Escape character is ^]\n"));
> -if (vshRunConsole(dom, name) == 0)
> +vshPrintExtra(ctl, _("Escape character is %s\n"), ctl->escapeChar);
This won't work. You want to undo the CONTROL() conversion, and output
a literal ^ followed by the counterpart character, as in:
vshPrintExtra(ctl, _("Escape character is ^%c\n"), CONTROL(ctl->escapeChar))
assuming that the control character is non-printable, and the
counterpart is printable. Maybe you need a c_isprint() call in there?
> @@ -16848,7 +16853,8 @@ vshUsage(void)
>"-t | --timing print timing
> information\n"
>"-l | --logoutput logging to file\n"
>"-v | --version[=short] program version\n"
> - "-V | --version=long version and full
> options\n\n"
> + "-V | --version=long version and full
> options\n"
> + "-e | --escape set escape sequence for
> console\n\n"
Well, we aren't very consistent in that formatting. I'd almost prefer
we change to a single format:
-l | --log=FILE log to FILE
-v short version
-V long version
--version[=TYPE] version, TYPE is short or long (default short)
-e | --escape=CHAR set console escape sequence to CHAR
--
Eric Blake ebl...@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] docs: fix grammar of capabilities
* docs/formatcaps.html.in: Avoid run-on sentence, wrap lines. --- Pushing under the trivial rule. docs/formatcaps.html.in | 45 ++--- 1 files changed, 26 insertions(+), 19 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index ce6f9a6..423bc48 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -64,25 +64,32 @@ BIOS you will see ... -The first block (in red) indicates the host hardware capabilities, currently -it is limited to the CPU properties and the power management features of -the host platform, but other information may be available, it shows the CPU architecture, -topology, model name, and additional features which are not included in the model but the -CPU provides them. Features of the chip are shown within the feature block (the block is -similar to what you will find in a Xen fully virtualized domain description). Further, -the power management features supported by the host are shown, such as Suspend-to-RAM (S3) -and Suspend-to-Disk (S4). In case the query for power management features succeeded but the -host does not support any such feature, then an empty-tag will be shown. Otherwise, if the query itself failed, no such tag will -be displayed (i.e., there will not be any power_management block or empty tag in the XML). -The second block (in blue) indicates the paravirtualization support of the -Xen support, you will see the os_type of xen to indicate a paravirtual -kernel, then architecture information and potential features. -The third block (in green) gives similar information but when running a -32 bit OS fully virtualized with Xen using the hvm support. -This section is likely to be updated and augmented in the future, see https://www.redhat.com/archives/libvir-list/2007-March/msg00215.html";>the -discussion which led to the capabilities format in the mailing-list -archives. +The first block (in red) indicates the host hardware + capabilities, such as CPU properties and the power + management features of the host platform. CPU models are + shown as additional features relative to the closest base + model, within a feature block (the block is similar to what + you will find in a Xen fully virtualized domain + description). Further, the power management features + supported by the host are shown, such as Suspend-to-RAM (S3) + and Suspend-to-Disk (S4). In case the query for power + management features succeeded but the host does not support + any such feature, then an empty + tag will be shown. Otherwise, if the query itself failed, no + such tag will be displayed (i.e., there will not be any + power_management block or empty tag in the XML). +The second block (in blue) indicates the paravirtualization + support of the Xen support, you will see the os_type of xen + to indicate a paravirtual kernel, then architecture + information and potential features. +The third block (in green) gives similar information but + when running a 32 bit OS fully virtualized with Xen using + the hvm support. +This section is likely to be updated and augmented in the + future, + see https://www.redhat.com/archives/libvir-list/2007-March/msg00215.html";>the + discussion which led to the capabilities format in the + mailing-list archives. -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/2] Implement the asynchronous suspend and RTC wakeup
On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote:
> Add the core functions that implement the functionality of the API.
> Suspend is done by using an asynchronous mechanism so that we can return
> the status to the caller successfully before the host gets suspended. This
> asynchronous operation is achieved by suspending the host in a separate
> thread of execution.
>
> +
> +#define MAX_SUSPEND_DURATION 365*24*60*60/* 1 year, a reasonable max? */
Resuming where I left off...
> +
> +/**
> + * setNodeWakeup:
> + * @alarmTime : time in seconds from now, at which the RTC alarm has to be
> set.
> + *
> + * Set up the RTC alarm to the specified time.
> + * Return 0 on success, -1 on failure.
> + */
> +
> +int setNodeWakeup(unsigned long long alarmTime)
> +{
> +virCommandPtr setAlarmCmd;
> +int ret = -1;
> +
> +if (alarmTime <= SUSPEND_DELAY || alarmTime > MAX_SUSPEND_DURATION)
Why is SUSPEND_DELAY in nodeinfo.h but MAX_SUSPEND_DURATION in the .c?
They might as well be next to one another.
> +return -1;
> +
> +setAlarmCmd = virCommandNewArgList("rtcwake", "-m", "no", "-s", NULL);
> +virCommandAddArgFormat(setAlarmCmd, "%lld", alarmTime);
'man rtcwake' says that not all systems support RTC wake from S4;
systems that have a functioning nvram-wakeup will succeed, but that is
not all systems. Do we need to be more cautious about allowing S4
suspension if we can't prove that RTC will wake up the system from S4?
On the other hand, you are using -m no to just set the wakeup time,
which ought to fail if the system does not support the requested delay
or the ability to wake up, so that you never actually suspend until
after you know the wakeup was successfully scheduled.
Hmm, does that mean the public API should provide a way to schedule the
wakeup without also scheduling a suspend?
> +++ b/src/util/threads-pthread.c
> @@ -81,10 +81,25 @@ void virMutexDestroy(virMutexPtr m)
> pthread_mutex_destroy(&m->lock);
> }
>
> -void virMutexLock(virMutexPtr m){
> +void virMutexLock(virMutexPtr m)
> +{
> pthread_mutex_lock(&m->lock);
> }
>
> +/**
> + * virMutexTryLock:
I'm not convinced we need this. As I understand it, your code does:
thread 1: thread 2: thread 3:
request suspend
grab lock
spawn helper
sleep 10 sec
return success
request suspend
lock not available
return failure
suspend
resume
request suspend
lock not available
return failure
release lock
But we don't need a try-lock operation, if we do:
thread 1: thread 2: thread 3:
request suspend
grab lock
request suspend
mark flag to true
release lock
grab lock
flag already true
release lock
return failure
spawn helper
sleep 10 sec
return success
suspend
resume
grab lock
flag already true
release lock
return failure
grab lock
clear flag
release lock
That is, instead of holding the lock for the entire duration of the
suspend, just hold the lock long enough to mark a volatile variable;
then you no longer need a non-blocking try-lock primitive, because the
lock will never starve anyone else long enough to be an issue.
But if you can still convince me that we need a try-lock operation, then
it should probably be added as a separate commit, alongside an
implementation for mingw in the same commit (without a mingw
implementation, you will cause a build failure for mingw32-libvirt).
> + * This is same as virMutexLock() except that
> + * if the mutex is unavailable (already locked),
> + * this fails and returns an error.
> + *
> + * Returns 1 if the lock was acquired, 0 if there was
> + * contention or error.
> + */
> +int virMutexTryLock(virMutexPtr m)
> +{
> + return !pthread_mutex_trylock(&m->lock);
Either return a bool (if we don't care about distinguishing why the lock
failed) or keep the return as an int but make it tri-state (1 for
grabbed, 0 for EBUSY, and -1 for all other failures (such as EINVAL).
> +++ b/src/util/threads.h
> @@ -81,6 +81,7 @@ int virMutexInitRecursive(virMutexPtr m)
> ATTRIBUTE_RETURN_CHECK;
> void virMutexDestroy(virMutexPtr m);
>
> void virMutexLock(virMutexPtr m);
> +int virMutexTryLock(virMutexPtr m);
And if you
Re: [libvirt] [PATCH v3 1/2] Implement the asynchronous suspend and RTC wakeup
On 11/22/2011 11:33 PM, Eric Blake wrote:
> On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote:
>> Add the core functions that implement the functionality of the API.
>> Suspend is done by using an asynchronous mechanism so that we can return
>> the status to the caller successfully before the host gets suspended. This
>> asynchronous operation is achieved by suspending the host in a separate
>> thread of execution.
>>
>> +
[...]
>
>> +return -1;
>> +
>> +setAlarmCmd = virCommandNewArgList("rtcwake", "-m", "no", "-s", NULL);
>> +virCommandAddArgFormat(setAlarmCmd, "%lld", alarmTime);
>
> 'man rtcwake' says that not all systems support RTC wake from S4;
> systems that have a functioning nvram-wakeup will succeed, but that is
> not all systems. Do we need to be more cautious about allowing S4
> suspension if we can't prove that RTC will wake up the system from S4?
>
> On the other hand, you are using -m no to just set the wakeup time,
> which ought to fail if the system does not support the requested delay
> or the ability to wake up, so that you never actually suspend until
> after you know the wakeup was successfully scheduled.
>
> Hmm, does that mean the public API should provide a way to schedule the
> wakeup without also scheduling a suspend?
But how would that help? The aim of having this API is to suspend and
resume the system.. and hence I don't see why it has to expose a
functionality to only schedule a wakeup..
>
>> +++ b/src/util/threads-pthread.c
>> @@ -81,10 +81,25 @@ void virMutexDestroy(virMutexPtr m)
>> pthread_mutex_destroy(&m->lock);
>> }
>>
>> -void virMutexLock(virMutexPtr m){
>> +void virMutexLock(virMutexPtr m)
>> +{
>> pthread_mutex_lock(&m->lock);
>> }
>>
>> +/**
>> + * virMutexTryLock:
>
> I'm not convinced we need this. As I understand it, your code does:
>
> thread 1: thread 2: thread 3:
> request suspend
> grab lock
> spawn helper
>sleep 10 sec
> return success
> request suspend
> lock not available
> return failure
>suspend
>resume
> request suspend
> lock not available
> return failure
>release lock
>
> But we don't need a try-lock operation, if we do:
>
> thread 1: thread 2: thread 3:
> request suspend
> grab lock
> request suspend
> mark flag to true
> release lock
> grab lock
> flag already true
> release lock
> return failure
> spawn helper
>sleep 10 sec
> return success
>suspend
>resume
> grab lock
> flag already true
> release lock
> return failure
>grab lock
>clear flag
>release lock
>
> That is, instead of holding the lock for the entire duration of the
> suspend, just hold the lock long enough to mark a volatile variable;
> then you no longer need a non-blocking try-lock primitive, because the
> lock will never starve anyone else long enough to be an issue.
>
Yes, that would work. (In fact, that was the way I first developed the
code. But then I felt trylock() was a fairly popular primitive to use
in such cases and hence I thought I might as well add it to libvirt).
Anyways, I am fine with going with the method you suggested above.
Thanks,
Srivatsa S. Bhat
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] Make the API public
On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote:
> Define the required interfaces to export the API.
>
> Signed-off-by: Srivatsa S. Bhat
> ---
>
> include/libvirt/libvirt.h.in |4
> src/driver.h |5
> src/libvirt.c| 48
> ++
> src/libvirt_public.syms |1 +
These changes are good for the first patch, in introducing the API.
Plus see my comment in 1/2 about the hunk defining VIR_S3 (or whatever
name we settle on) being part of this patch.
> src/qemu/qemu_driver.c |1 +
This change should be shuffled into the qemu driver implementation of
the driver backend.
> src/remote/remote_driver.c |1 +
> src/remote/remote_protocol.x | 12 ++-
These changes belong to a second patch, implementing the RPC driver
backend. And don't forget a patch to virsh.
> +++ b/include/libvirt/libvirt.h.in
> @@ -1055,6 +1055,10 @@ unsigned long long virNodeGetFreeMemory
> (virConnectPtr conn);
> int virNodeGetSecurityModel (virConnectPtr conn,
> virSecurityModelPtr
> secmodel);
>
> +int virNodeSuspendForDuration (virConnectPtr conn,
> + int state,
> + unsigned long long
> duration);
Needs an 'unsigned int flags' argument, even if we always pass 0 for now.
> +++ b/src/libvirt.c
> @@ -6303,6 +6303,54 @@ error:
> }
>
> /**
> + * virNodeSuspendForDuration:
> + * @conn: pointer to the hypervisor connection
> + * @state: the state to which the host must be suspended to,
> + * such as: VIR_S3 (Suspend-to-RAM)
> + * VIR_S4 (Suspend-to-Disk)
Is this a bitmask or a linear list? I guess a list makes more sense.
Also, some machines support a hybrid between S3 and S4 (pm-is-supported
--suspend-hybrid), which saves state to disk like S4 but then drops to
S3 for faster resume as long as power is present. Perhaps a hybrid
state is deserving of a third value in the enum?
> + * @duration: the time duration in seconds, for which the host
> + *has to be suspended
> + *
> + * Suspend the node (host machine) for the given duration of time
> + * in the specified state (such as S3 or S4). Resume the node
> + * after the time duration is complete.
We might want to be a bit more "fuzzy" on how we word this, so that we
aren't making impossible promises. Something like:
Attempt to suspend the node (host machine) for the given duration of
time into the specified state (S3 for suspend to RAM, S4 for suspend to
disk). Schedule the node's real-time-clock interrupt to resume the node
after the time duration is complete.
> + *
> + * Returns 0 on success (i.e., the node will be suspended after a
> + * short delay), -1 on failure (the operation is not supported).
(the operation is not supported, or an attempted suspend is already
underway).
> + */
> +int
> +virNodeSuspendForDuration(virConnectPtr conn,
> + int state,
> + unsigned long long duration)
> +{
> +
> +VIR_DEBUG("conn=%p, state=%d, duration=%lld", conn, state, duration);
> +
> +virResetLastError();
> +
> +if (!VIR_IS_CONNECT(conn)) {
> +virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> +virDispatchError(NULL);
> +return -1;
> +}
> +
You must check for and reject an attempt to use this on a read-only
connection.
> +if (conn->driver->nodeSuspendForDuration) {
> +int ret;
> +ret = conn->driver->nodeSuspendForDuration(conn, state, duration);
Be sure to pass a flags argument on to the driver.
> +if (ret < 0)
> +goto error;
> +return ret;
> +}
> +
> +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +virDispatchError(conn);
> +return -1;
> +}
> +
> +
> +/**
> * virDomainGetSchedulerType:
> * @domain: pointer to domain object
> * @nparams: pointer to number of scheduler parameters, can be NULL
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index bcefb10..fd44170 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -478,6 +478,7 @@ LIBVIRT_0.9.4 {
> virDomainGetBlockJobInfo;
> virDomainBlockJobSetSpeed;
> virDomainBlockPull;
> +virNodeSuspendForDuration;
> } LIBVIRT_0.9.3;
The new API must be in a LIBVIRT_0.9.8 section at the bottom of the
file; we are NOT re-doing 0.9.3.
>
> LIBVIRT_0.9.5 {
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b4dc582..f744539 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10911,6 +10911,7 @@ static virDriver qemuDriver = {
> .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */
> .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
> .domainBlockPull
Re: [libvirt] [PATCH V2 1/5] Add a mac chain
On 11/22/2011 08:51 AM, Stefan Berger wrote:
> With hunks borrowed from one of David Steven's previous patches, we now
> add the capability of having a 'mac' chain which is useful to filter
> for multiple valid MAC addresses.
>
> Signed-off-by: David L Stevens
> Signed-off-by: Stefan Berger
>
> ---
> docs/schemas/nwfilter.rng |3 +++
> src/conf/nwfilter_conf.c |2 ++
> src/conf/nwfilter_conf.h |2 ++
> src/nwfilter/nwfilter_ebiptables_driver.c | 22 --
> 4 files changed, 27 insertions(+), 2 deletions(-)
Almost - in addressing my v1 comments, you introduced some other problems.
> @@ -2806,11 +2808,25 @@ ebtablesCreateTmpSubChain(ebiptablesRule
> char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
> char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
>: CHAINPREFIX_HOST_OUT_TEMP;
> +char *protostr = NULL;
>
> PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
> PRINT_CHAIN(chain, chainPrefix, ifname,
> (filtername) ? filtername : l3_protocols[protoidx].val);
>
> +switch (protoidx) {
> +case L2_PROTO_MAC_IDX:
> +break;
> +default:
> +virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr);
> +break;
> +}
> +
> +if (!protostr) {
> +virReportOOMError();
Oops. This gives a spurious OOM failure if protoidx is L2_PROTO_MAC_IDX.
> @@ -2819,7 +2835,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
>CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
>CMD_EXEC
>"%s"
> - CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
> + CMD_DEF("%s -t %s -%%c %s %%s %s -j %s")
^^
While you fixed the double space problem for a non-empty protostr, you
still have the double space for L2_PROTO_MAC_IDX. To completely avoid a
double space, as well as the spurious OOM, you'd need:
case L2_PROTO_MAC_IDX:
protostr = strdup("");
break;
default:
virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr);
break;
...
CMD_DEF("%s -t %s -%%c %s %%s %s-j %s")
ACK with those tweaks.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] rpc: Add some debug messages to virNetClient
On Tue, Nov 22, 2011 at 04:32:34PM +0100, Jiri Denemark wrote:
> ---
> src/rpc/virnetclient.c |4
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index c99e87c..025d270 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -1040,6 +1040,7 @@ static bool
> virNetClientIOEventLoopRemoveDone(virNetClientCallPtr call,
> VIR_DEBUG("Waking up sleep %p", call);
> virCondSignal(&call->cond);
> } else {
> +VIR_DEBUG("Removing completed call %p", call);
> if (call->expectReply)
> VIR_WARN("Got a call expecting a reply but without a waiting
> thread");
> ignore_value(virCondDestroy(&call->cond));
> @@ -1070,6 +1071,8 @@ static bool
> virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call,
> if (call->haveThread) {
> VIR_DEBUG("Waking up sleep %p", call);
> virCondSignal(&call->cond);
> +} else {
> +VIR_DEBUG("Keeping unfinished call %p in the list", call);
> }
> return false;
> } else {
> @@ -1081,6 +1084,7 @@ static bool
> virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call,
> VIR_DEBUG("Waking up sleep %p", call);
> virCondSignal(&call->cond);
> } else {
> +VIR_DEBUG("Removing call %p", call);
> if (call->expectReply)
> VIR_WARN("Got a call expecting a reply but without a waiting
> thread");
> ignore_value(virCondDestroy(&call->cond));
ACK
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] rpc: Fix a typo in virNetClientSendNonBlock documentation
On Tue, Nov 22, 2011 at 04:32:32PM +0100, Jiri Denemark wrote: > --- > src/rpc/virnetclient.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c > index b518abd..0effceb 100644 > --- a/src/rpc/virnetclient.c > +++ b/src/rpc/virnetclient.c > @@ -1595,7 +1595,7 @@ int virNetClientSendNoReply(virNetClientPtr client, > * Send a message asynchronously, without any reply > * > * The caller is responsible for free'ing @msg, *except* if > - * this method returns -1. > + * this method returns 1. > * > * Returns 2 on full send, 1 on partial send, 0 on no send, -1 on error > */ ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 2/5] Add support for STP filtering
On 11/22/2011 08:51 AM, Stefan Berger wrote:
> This patch adds support for filtering of STP (spanning tree protocol) traffic
> to the parser and makes us of the ebtables support for STP filtering. This
> code
> now enables the filtering of traffic in chains with prefix 'stp'.
>
> Signed-off-by: Stefan Berger
>
> ---
>
> v2:
> - addressing Eric Blake's comments
> - replacing occurrences of number[20] with number[INT_BUFFER_BOUND(uint32)];
> this
>also works for IP masks which are expressed as "%d".
The maximum %d is one byte longer than %u, thanks to a negative sign; so
you've got a potential off-by-one issue (although a negative IP mask
doesn't make sense, so you may be safe for now).
>
> +static const virXMLAttr2Struct stpAttributes[] = {
> +/* spanning tree uses a special destination MAC address */
> +{
I'd feel a bit better if this comment appears near here:
/* STP is documented by IEEE 802.1D; for a synopsis,
* see http://www.javvin.com/protocolSTP.html */
> @@ -937,7 +950,7 @@ iptablesHandleIpHdr(virBufferPtr buf,
> virBufferPtr prefix)
> {
> char ipaddr[INET6_ADDRSTRLEN],
> - number[20];
> + number[INT_BUFSIZE_BOUND(uint32_t)];
Maybe it's best to rely on util.h giving us MAX, and doing:
char number[MAX(INT_BUFSIZE_BOUND(uint32_t),
INT_BUFSIZE_BOUND(int))];
so that we can use both %u for uint32_t, and %d for int, without
worrying about any other weird type promotions going on.
ACK with those nits addressed.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] rpc: Fix handling of non-blocking calls that could not be sent
On Tue, Nov 22, 2011 at 04:32:33PM +0100, Jiri Denemark wrote:
> When virNetClientIOEventLoop is called for a non-blocking call and not
> even a single byte can be sent from this call without blocking, we
> properly reported that to the caller which properly frees the call. But
> we never removed the call from a call queue.
> ---
> src/rpc/virnetclient.c |7 ++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index 0effceb..c99e87c 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -1256,7 +1256,12 @@ static int virNetClientIOEventLoop(virNetClientPtr
> client,
> /* We're not done, but we're non-blocking */
> if (thiscall->nonBlock) {
> virNetClientIOEventLoopPassTheBuck(client, thiscall);
> -return thiscall->sentSomeData ? 1 : 0;
> +if (thiscall->sentSomeData) {
> +return 1;
> +} else {
> +virNetClientCallRemove(&client->waitDispatch, thiscall);
> +return 0;
> +}
> }
>
> if (fds[0].revents & (POLLHUP | POLLERR)) {
ACK
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] rpc: Pass the buck only to the first available thread
On Tue, Nov 22, 2011 at 04:32:31PM +0100, Jiri Denemark wrote:
> ---
> src/rpc/virnetclient.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index deeeaad..b518abd 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -1102,7 +1102,7 @@ static void
> virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli
> if (tmp != thiscall && tmp->haveThread) {
> VIR_DEBUG("Passing the buck to %p", tmp);
> virCondSignal(&tmp->cond);
> -break;
> +return;
> }
> tmp = tmp->next;
> }
ACK
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 1/5] Add a mac chain
On 11/22/2011 01:47 PM, Eric Blake wrote:
On 11/22/2011 08:51 AM, Stefan Berger wrote:
With hunks borrowed from one of David Steven's previous patches, we now
add the capability of having a 'mac' chain which is useful to filter
for multiple valid MAC addresses.
Signed-off-by: David L Stevens
Signed-off-by: Stefan Berger
---
docs/schemas/nwfilter.rng |3 +++
src/conf/nwfilter_conf.c |2 ++
src/conf/nwfilter_conf.h |2 ++
src/nwfilter/nwfilter_ebiptables_driver.c | 22 --
4 files changed, 27 insertions(+), 2 deletions(-)
Almost - in addressing my v1 comments, you introduced some other problems.
@@ -2806,11 +2808,25 @@ ebtablesCreateTmpSubChain(ebiptablesRule
char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
: CHAINPREFIX_HOST_OUT_TEMP;
+char *protostr = NULL;
PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
PRINT_CHAIN(chain, chainPrefix, ifname,
(filtername) ? filtername : l3_protocols[protoidx].val);
+switch (protoidx) {
+case L2_PROTO_MAC_IDX:
+break;
+default:
+virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr);
+break;
+}
+
+if (!protostr) {
+virReportOOMError();
Oops. This gives a spurious OOM failure if protoidx is L2_PROTO_MAC_IDX.
@@ -2819,7 +2835,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
CMD_EXEC
"%s"
- CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
+ CMD_DEF("%s -t %s -%%c %s %%s %s -j %s")
^^
While you fixed the double space problem for a non-empty protostr, you
still have the double space for L2_PROTO_MAC_IDX. To completely avoid a
double space, as well as the spurious OOM, you'd need:
case L2_PROTO_MAC_IDX:
protostr = strdup("");
break;
My further testing also pointed that out to me in the meantime... :-/
default:
virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr);
I removed the trailing whitespace above...
break;
...
CMD_DEF("%s -t %s -%%c %s %%s %s-j %s")
ACK with those tweaks.
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
