Re: [libvirt] [test-API][PATCH 1/2] update env_clear for new way of function reference
ACK pls also push last series patch of clean work. Thanks! Best Regards! Wayne Sun Redhat QE, Beijing, China +86-10-6260-8246 - Original Message - From: Guannan Ren g...@redhat.com To: libvir-list@redhat.com Sent: Friday, November 11, 2011 11:21:27 AM Subject: [libvirt] [test-API][PATCH 1/2] update env_clear for new way of function reference --- env_clear.py | 23 ++- 1 files changed, 10 insertions(+), 13 deletions(-) diff --git a/env_clear.py b/env_clear.py index 302ff62..5e78ab5 100644 --- a/env_clear.py +++ b/env_clear.py @@ -14,7 +14,7 @@ # distribution and at http://www.gnu.org/licenses. # # This module matches the reference of clearing function from each testcase -# to the corresponding testcase's argument in the order of testcase running +# to the corresponding testcase's argument in the order of testcase running # import mapper @@ -28,17 +28,17 @@ class EnvClear(object): self.cases_clearfunc_ref_dict = cases_clearfunc_ref_dict self.logfile = logfile self.loglevel = loglevel - + mapper_obj = mapper.Mapper(activity) -pkg_tripped_cases = mapper_obj.get_package_tripped() +clean_pkg_casename_func = mapper_obj.clean_package_casename_func_map() self.cases_ref_names = [] -for case in pkg_tripped_cases: +for case in clean_pkg_casename_func: case_ref_name = case.keys()[0] self.cases_ref_names.append(case_ref_name) self.cases_params_list = [] -for case in pkg_tripped_cases: +for case in clean_pkg_casename_func: case_params = case.values()[0] self.cases_params_list.append(case_params) @@ -48,10 +48,10 @@ class EnvClear(object): def env_clear(self): Run each clearing function with the corresponding arguments - + envlog = log.EnvLog(self.logfile, self.loglevel) logger = envlog.env_log() - + testcase_number = len(self.cases_ref_names) for i in range(testcase_number): @@ -59,12 +59,9 @@ class EnvClear(object): case_ref_name = self.cases_ref_names[i] case_params = self.cases_params_list[i] -if case_ref_name == 'sleep': -continue -else: -case_params['logger'] = logger -self.cases_clearfunc_ref_dict[case_ref_name](case_params) +case_params['logger'] = logger +self.cases_clearfunc_ref_dict[case_ref_name](case_params) del envlog -return 0 +return 0 -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH 2/2] update proxy and mapper part for cleanup using new reference
ACK
Thanks!
Best Regards!
Wayne Sun
Redhat QE, Beijing, China
+86-10-6260-8246
- Original Message -
From: Guannan Ren g...@redhat.com
To: libvir-list@redhat.com
Sent: Friday, November 11, 2011 11:21:28 AM
Subject: [libvirt] [test-API][PATCH 2/2] update proxy and mapper part for
cleanup using new reference
---
mapper.py | 19 +++
proxy.py | 11 +--
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/mapper.py b/mapper.py
index affc510..f0b675a 100644
--- a/mapper.py
+++ b/mapper.py
@@ -55,3 +55,22 @@ class Mapper(object):
prev_testcases_params = testcases_params
return tripped_cases_list
+
+def clean_package_casename_func_map(self):
+get testcase function maping without cleaning ones
+tripped_cases_list = []
+for testcase in self.testcases_list:
+tripped_case = {}
+testcase_name = testcase.keys()[0]
+if : in testcase_name:
+casename = testcase_name.split(:)[1]
+func = casename + _clean
+
+if testcase_name == 'sleep' or testcase_name == 'clean':
+continue
+
+testcases_params = testcase.values()[0]
+tripped_case[testcase_name + : + func] = testcases_params
+tripped_cases_list.append(tripped_case)
+
+return tripped_cases_list
diff --git a/proxy.py b/proxy.py
index f3664e3..aa34d9a 100644
--- a/proxy.py
+++ b/proxy.py
@@ -25,9 +25,10 @@ class Proxy(object):
def __init__(self, testcases_names):
Argument case_list is test case list
self.testcases_names = testcases_names
-self.func_dict = dict()
def get_func_call_dict(self):
+Return running function reference dictionary
+self.func_dict = dict()
for testcase_name in self.testcases_names:
# Get programming package, casename
elements = testcase_name.split(:)
@@ -51,8 +52,14 @@ class Proxy(object):
def get_clearfunc_call_dict(self):
Return a clearing function reference dictionary.
+self.func_dict = dict()
for testcase_name in self.testcases_names:
# Get programming package, casename
+elements = testcase_name.split(:)
+
+if len(elements) == 3:
+continue
+
package = testcase_name.split(:)[0]
casename = testcase_name.split(:)[1]
@@ -64,7 +71,7 @@ class Proxy(object):
func_ref = funcs(package, casename, func)
# Construct function call dictionary
-key = package + : + casename
+key = package + : + casename + : + func
self.func_dict[key] = func_ref
return self.func_dict
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] make install libvirt failled.
于 2011年11月14日 12:42, ShaoHe Feng 写道: I make install libvirt failed on my Ubuntu 11.11. I think some html dependent packages are missed on my build environment. Does anyone know the reason? Thanks. You need to install xhtml1-dtds Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-glib 2/3] Add GVirDomainInterface
On Thu, Nov 10, 2011 at 04:15:01PM -0500, Marc-André Lureau wrote: On Thu, Nov 10, 2011 at 09:33:42PM +0100, Marc-André Lureau wrote: Do we really need to use GSlice here? I consider GSlice as something to use when you want to make many allocations of same-size objects, will we allocate many of these stats objects? Yes, it's frequently allocated (1/second), and kept in memory too for history. Ok, fine with me then. * how do we handle ABI compatibility the day we need to add fields to this struct? It's only a returned structure, allocated by the lib, so I guess that's fine to append fields later, right? Yes, I think it should be fine, I think it's possible to find scenarios that could break when adding fields, but that's probably some convoluted cases where we can say don't do that. Iirc libvirt does this too, so this should be good enough for us :) Christophe pgpotI64sXtjH.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 1/3] Add GVirDomainDevice abstract class
Hey, On Thu, Nov 10, 2011 at 04:10:32PM -0500, Marc-André Lureau wrote: - Mensaje original - +G_GNUC_INTERNAL +virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self) G_GNUC_INTERNAL isn't needed here since it's the default setting for symbols not listed in the .sym file. I think it's better to be explicit about it. It's a way to declare in the code that this function is private, without having to look into the sym file. I wish we could do the same for the rest of the prviate functions. Given that having it or not will have exactly the same result, I have doubts we'll consistently mark all private functions with G_GNUC_INTERNAL. Only having some of the private functions marked with it, and the others without the marking will only create more confusion. If we are careful about marking all private functions with G_GNUC_INTERNAL, then why not. Christophe pgp7Cgvgyd0S1.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
Am 12.11.2011 11:25, schrieb Avi Kivity: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? Not sure what results and what computation you mean, but let me clarify a bit about bdrv_reopen: The main purpose of bdrv_reopen() is to change flags, for example toggle O_SYNC during runtime in order to allow the guest to toggle WCE. This doesn't necessarily mean a close()/open() sequence if there are other means to change the flags, like fcntl() (or even using other protocols than files). The idea here was to extend this to invalidate all caches if some specific flag is set. As you don't change any other flag, this will usually not be a reopen on a lower level. If we need to use open() though, and it fails (this is really the only different result that comes to mind) then bdrv_reopen() would fail and the old fd would stay in use. Migration would have to fail, but I don't think this case is ever needed for reopening after migration. What's wrong with just delaying the open? Nothing, except that with today's code it's harder to do. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 10:58:16AM +0100, Kevin Wolf wrote: Am 12.11.2011 11:25, schrieb Avi Kivity: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? Not sure what results and what computation you mean, but let me clarify a bit about bdrv_reopen: The main purpose of bdrv_reopen() is to change flags, for example toggle O_SYNC during runtime in order to allow the guest to toggle WCE. This doesn't necessarily mean a close()/open() sequence if there are other means to change the flags, like fcntl() (or even using other protocols than files). The idea here was to extend this to invalidate all caches if some specific flag is set. As you don't change any other flag, this will usually not be a reopen on a lower level. If we need to use open() though, and it fails (this is really the only different result that comes to mind) then bdrv_reopen() would fail and the old fd would stay in use. Migration would have to fail, but I don't think this case is ever needed for reopening after migration. What's wrong with just delaying the open? Nothing, except that with today's code it's harder to do. Kevin It seems cleaner, though, doesn't it? -- MST -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. -- |: 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/3] Add GVirDomainInterface
Hey,
A few more comments I made on IRC (no access to this email when I reviewed
the patches), sending them here to be sure they are not missed/lost. The
same comments apply to patch 3/3.
Can you resend the 3 patches with the various issues fixed? then I'll ACK
them.
Christophe
On Thu, Nov 10, 2011 at 09:33:42PM +0100, Marc-André Lureau wrote:
diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.c
b/libvirt-gobject/libvirt-gobject-domain-interface.c
new file mode 100644
index 000..65a5467
--- /dev/null
+++ b/libvirt-gobject/libvirt-gobject-domain-interface.c
@@ -0,0 +1,213 @@
+/*
+ * libvirt-gobject-domain-interface.c: 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
+ *
+ * Author: Marc-André Lureau marcandre.lur...@redhat.com
+ */
+
+#include config.h
+
+#include libvirt/virterror.h
+#include string.h
+
+#include libvirt-glib/libvirt-glib.h
+#include libvirt-gobject/libvirt-gobject.h
+
+#include libvirt-gobject/libvirt-gobject-domain-device-private.h
+
+extern gboolean debugFlag;
+
+#define DEBUG(fmt, ...) do { if (G_UNLIKELY(debugFlag)) g_debug(fmt, ##
__VA_ARGS__); } while (0)
+
+#define GVIR_DOMAIN_INTERFACE_GET_PRIVATE(obj) \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_INTERFACE,
GVirDomainInterfacePrivate))
+
+struct _GVirDomainInterfacePrivate
+{
+gchar *path;
+};
+
+G_DEFINE_TYPE(GVirDomainInterface, gvir_domain_interface,
GVIR_TYPE_DOMAIN_DEVICE);
+
+enum {
+PROP_0,
+PROP_PATH,
+};
+
+#define GVIR_DOMAIN_INTERFACE_ERROR gvir_domain_interface_error_quark()
+
+
+static GQuark
+gvir_domain_interface_error_quark(void)
+{
+return g_quark_from_static_string(gvir-domain-interface);
+}
+
+static void gvir_domain_interface_get_property(GObject *object,
+ guint prop_id,
+ GValue *value,
+ GParamSpec *pspec)
+{
+GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object);
+GVirDomainInterfacePrivate *priv = self-priv;
+
+switch (prop_id) {
+case PROP_PATH:
+g_value_set_string(value, priv-path);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+
+static void gvir_domain_interface_set_property(GObject *object,
+ guint prop_id,
+ const GValue *value,
+ GParamSpec *pspec)
+{
+GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object);
+GVirDomainInterfacePrivate *priv = self-priv;
+
+switch (prop_id) {
+case PROP_PATH:
+if (priv-path)
+g_free(priv-path);
+priv-path = g_value_dup_string(value);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+
+static void gvir_domain_interface_finalize(GObject *object)
+{
+GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object);
+GVirDomainInterfacePrivate *priv = self-priv;
+
+DEBUG(Finalize GVirDomainInterface=%p, self);
+
+if (priv-path)
+g_free(priv-path);
+
+G_OBJECT_CLASS(gvir_domain_interface_parent_class)-finalize(object);
+}
+
+static void gvir_domain_interface_class_init(GVirDomainInterfaceClass *klass)
+{
+GObjectClass *object_class = G_OBJECT_CLASS (klass);
+
+object_class-finalize = gvir_domain_interface_finalize;
+object_class-get_property = gvir_domain_interface_get_property;
+object_class-set_property = gvir_domain_interface_set_property;
+
+g_object_class_install_property(object_class,
+PROP_PATH,
+g_param_spec_string(path,
+Path,
+The interface path,
+NULL,
+G_PARAM_READWRITE |
+
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? I'm asking because for avoiding the former, things like access() could be enough, whereas for the latter we'd have to do a full open. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 11:08:02AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Regards, Daniel IIUC, the 'cont' that we were discussing is the startup of the VM at destination after migration completes. A failure results in migration failure, which libvirt has been able to handle since forever. In case of the 'cont' command on source upon migration failure, qemu was running there previously so it's likely configuration is OK. Am I confused? If no, libvirt seems unaffected. -- |: 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. Daniel Do you run qemu with -S, then give a 'cont' command to start it? -- |: 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. Do you run qemu with -S, then give a 'cont' command to start it? Yes 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote: Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. If started correctly QEMU should not report them to the guest OS, but pause instead. -- Gleb. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 11:37:27AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. Do you run qemu with -S, then give a 'cont' command to start it? Yes Daniel OK, so let's go back one step now - how is this related to 'rollback to source host'? -- MST -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 11:37:27AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. Do you run qemu with -S, then give a 'cont' command to start it? Yes Daniel Probably in an attempt to improve reliability :) So this is in fact unrelated to migration. So we can either ignore this bug (assuming no distros ship cutting edge qemu with an old libvirt), or special-case -S and do an open/close cycle on startup. -- |: 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 01:56:36PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 11:37:27AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. Do you run qemu with -S, then give a 'cont' command to start it? Yes OK, so let's go back one step now - how is this related to 'rollback to source host'? In the old libvirt migration protocol, by the time we run 'cont' on the destination, the source QEMU has already been killed off, so there's nothing to resume on failure. 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] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 01:51:40PM +0200, Michael S. Tsirkin wrote:
On Mon, Nov 14, 2011 at 11:37:27AM +, Daniel P. Berrange wrote:
On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote:
On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote:
On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote:
Am 14.11.2011 12:08, schrieb Daniel P. Berrange:
On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote:
On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote:
On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote:
On 11/11/2011 12:15 PM, Kevin Wolf wrote:
Am 10.11.2011 22:30, schrieb Anthony Liguori:
Live migration with qcow2 or any other image format is just
not going to work
right now even with proper clustered storage. I think doing a
block level flush
cache interface and letting block devices decide how to do it
is the best approach.
I would really prefer reusing the existing open/close code. It
means
less (duplicated) code, is existing code that is well tested
and doesn't
make migration much of a special case.
If you want to avoid reopening the file on the OS level, we can
reopen
only the topmost layer (i.e. the format, but not the protocol)
for now
and in 1.1 we can use bdrv_reopen().
Intuitively I dislike _reopen style interfaces. If the second
open
yields different results from the first, does it invalidate any
computations in between?
What's wrong with just delaying the open?
If you delay the 'open' until the mgmt app issues 'cont', then
you loose
the ability to rollback to the source host upon open failure for
most
deployed versions of libvirt. We only fairly recently switched to
a five
stage migration handshake to cope with rollback when 'cont' fails.
Daniel
I guess reopen can fail as well, so this seems to me to be an
important
fix but not a blocker.
If if the initial open succeeds, then it is far more likely that a
later
re-open will succeed too, because you have already elminated the
possibility
of configuration mistakes, and will have caught most storage
runtime errors
too. So there is a very significant difference in reliability
between doing
an 'open at startup + reopen at cont' vs just 'open at cont'
Based on the bug reports I see, we want to be very good at
detecting and
gracefully handling open errors because they are pretty frequent.
Do you have some more details on the kind of errors? Missing files,
permissions, something like this? Or rather something related to the
actual content of an image file?
Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI
setup. Access permissions due to incorrect user / group setup, or read
only mounts, or SELinux denials. Actual I/O errors are less common and
are not so likely to cause QEMU to fail to start any, since QEMU is
likely to just report them to the guest OS instead.
Do you run qemu with -S, then give a 'cont' command to start it?
Probably in an attempt to improve reliability :)
Not really. We can't simply let QEMU start its own CPUs, because there are
various tasks that need performing after the migration transfer finishes,
but before the CPUs are allowed to be started. eg
- Finish 802.11Qb{g,h} (VEPA) network port profile association on target
- Release leases for any resources associated with the source QEMU
via a configured lock manager (eg sanlock)
- Acquire leases for any resources associated with the target QEMU
via a configured lock manager (eg sanlock)
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 6/6 (v2)] Allow non-blocking message sending on virNetClient
From: Daniel P. Berrange berra...@redhat.com
Split the existing virNetClientSend into two parts
virNetClientSend and virNetClientSendNoReply, instead
of having a 'bool expectReply' parameter.
Add a new virNetClientSendNonBlock which returns 2 on
full send, 1 on partial send, 0 on no send, -1 on error
If a partial send occurs, then a subsequent call to any
of the virNetClientSend* APIs will finish any outstanding
I/O.
TODO: the virNetClientEvent event handler could be used
to speed up completion of partial sends if an event loop
is present.
In v2:
- Fix logic in virNetClientIOEventLoopRemoveNonBlocking
* src/rpc/virnetclientprogram.c, src/rpc/virnetclientstream.c:
Update for changed API
---
src/rpc/virnetclient.c | 200 +++-
src/rpc/virnetclient.h |4 +
src/rpc/virnetsocket.c | 13 +++
src/rpc/virnetsocket.h |1 +
4 files changed, 200 insertions(+), 18 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 96d1886..17105fe 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -55,6 +55,9 @@ struct _virNetClientCall {
virNetMessagePtr msg;
bool expectReply;
+bool nonBlock;
+bool haveThread;
+bool sentSomeData;
virCond cond;
@@ -86,7 +89,12 @@ struct _virNetClient {
int wakeupSendFD;
int wakeupReadFD;
-/* List of threads currently waiting for dispatch */
+/*
+ * List of calls currently waiting for dispatch
+ * The calls should all have threads waiting for
+ * them, except possibly the first call in the list
+ * which might be a partially sent non-blocking call.
+ */
virNetClientCallPtr waitDispatch;
/* True if a thread holds the buck */
bool haveTheBuck;
@@ -648,7 +656,7 @@ virNetClientCallDispatchReply(virNetClientPtr client)
virNetClientCallPtr thecall;
/* Ok, definitely got an RPC reply now find
- out who's been waiting for it */
+ out which waiting call is associated with it */
thecall = client-waitDispatch;
while (thecall
!(thecall-msg-header.prog == client-msg.header.prog
@@ -824,6 +832,8 @@ virNetClientIOWriteMessage(virNetClientPtr client,
ret = virNetSocketWrite(client-sock,
thecall-msg-buffer +
thecall-msg-bufferOffset,
thecall-msg-bufferLength -
thecall-msg-bufferOffset);
+if (ret || virNetSocketHasPendingData(client-sock))
+thecall-sentSomeData = true;
if (ret = 0)
return ret;
@@ -1015,17 +1025,69 @@ static bool
virNetClientIOEventLoopRemoveDone(virNetClientCallPtr call,
return false;
/*
- * ...they won't actually wakeup until
+ * ...if the call being removed from the list
+ * still has a thread, then wake that thread up,
+ * otherwise free the call. The latter should
+ * only happen for calls without replies.
+ *
+ * ...the threads won't actually wakeup until
* we release our mutex a short while
* later...
*/
-VIR_DEBUG(Waking up sleeping call %p, call);
-virCondSignal(call-cond);
+if (call-haveThread) {
+VIR_DEBUG(Waking up sleep %p, call);
+virCondSignal(call-cond);
+} else {
+if (call-expectReply)
+VIR_WARN(Got a call expecting a reply but without a waiting
thread);
+ignore_value(virCondDestroy(call-cond));
+VIR_FREE(call);
+}
return true;
}
+static bool virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call,
+ void *opaque)
+{
+virNetClientCallPtr thiscall = opaque;
+
+if (call == thiscall)
+return false;
+
+if (!call-nonBlock)
+return false;
+
+if (call-sentSomeData) {
+/*
+ * If some data has been sent we must keep it in the list,
+ * but still wakeup any thread
+ */
+if (call-haveThread) {
+VIR_DEBUG(Waking up sleep %p, call);
+virCondSignal(call-cond);
+}
+return false;
+} else {
+/*
+ * If no data has been sent, we can remove it from the list.
+ * Wakup any thread, otherwise free the caller ourselves
+ */
+if (call-haveThread) {
+VIR_DEBUG(Waking up sleep %p, call);
+virCondSignal(call-cond);
+} else {
+if (call-expectReply)
+VIR_WARN(Got a call expecting a reply but without a waiting
thread);
+ignore_value(virCondDestroy(call-cond));
+VIR_FREE(call);
+}
+return true;
+}
+}
+
+
static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client,
virNetClientCallPtr thiscall)
{
VIR_DEBUG(Giving up the buck %p, thiscall);
@@ -1033,19 +1095,29 @@ static void
virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli
/* See if someone else is
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On Mon, Nov 14, 2011 at 11:58:14AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 01:56:36PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 11:37:27AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 11:29:18AM +, Daniel P. Berrange wrote: On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: Am 14.11.2011 12:08, schrieb Daniel P. Berrange: On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 14, 2011 at 10:16:10AM +, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel I guess reopen can fail as well, so this seems to me to be an important fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Do you have some more details on the kind of errors? Missing files, permissions, something like this? Or rather something related to the actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. Do you run qemu with -S, then give a 'cont' command to start it? Yes OK, so let's go back one step now - how is this related to 'rollback to source host'? In the old libvirt migration protocol, by the time we run 'cont' on the destination, the source QEMU has already been killed off, so there's nothing to resume on failure. Daniel I see. So again there are two solutions I see: 1. ignore old libvirt as it can't restart source reliably anyway 2. open files when migration is completed (after startup, but before cont) -- |: 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 1/6] Add GVirDomainDevice abstract class
---
libvirt-gobject/Makefile.am|3 +
.../libvirt-gobject-domain-device-private.h| 31 +
libvirt-gobject/libvirt-gobject-domain-device.c| 139
libvirt-gobject/libvirt-gobject-domain-device.h| 64 +
libvirt-gobject/libvirt-gobject.h |1 +
libvirt-gobject/libvirt-gobject.sym|2 +
6 files changed, 240 insertions(+), 0 deletions(-)
create mode 100644 libvirt-gobject/libvirt-gobject-domain-device-private.h
create mode 100644 libvirt-gobject/libvirt-gobject-domain-device.c
create mode 100644 libvirt-gobject/libvirt-gobject-domain-device.h
diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am
index 4f84b8b..56a047e 100644
--- a/libvirt-gobject/Makefile.am
+++ b/libvirt-gobject/Makefile.am
@@ -7,6 +7,7 @@ GOBJECT_HEADER_FILES = \
libvirt-gobject.h \
libvirt-gobject-main.h \
libvirt-gobject-domain-snapshot.h \
+ libvirt-gobject-domain-device.h \
libvirt-gobject-domain.h \
libvirt-gobject-interface.h \
libvirt-gobject-network.h \
@@ -21,6 +22,7 @@ GOBJECT_HEADER_FILES = \
GOBJECT_SOURCE_FILES = \
libvirt-gobject-main.c \
libvirt-gobject-domain-snapshot.c \
+ libvirt-gobject-domain-device.c \
libvirt-gobject-domain.c \
libvirt-gobject-interface.c \
libvirt-gobject-network.c \
@@ -42,6 +44,7 @@ libvirt_gobject_1_0_la_HEADERS = \
$(GOBJECT_HEADER_FILES) \
libvirt-gobject-input-stream.h
nodist_libvirt_gobject_1_0_la_HEADERS = \
+ libvirt-gobject-domain-device-private.h \
libvirt-gobject-enums.h
libvirt_gobject_1_0_la_SOURCES = \
$(libvirt_gobject_1_0_la_HEADERS) \
diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h
b/libvirt-gobject/libvirt-gobject-domain-device-private.h
new file mode 100644
index 000..2a34309
--- /dev/null
+++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h
@@ -0,0 +1,31 @@
+/*
+ * libvirt-gobject-domain-device-private.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
+ *
+ * Author: Marc-André Lureau marcandre.lur...@redhat.com
+ */
+#ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_PRIVATE_H__
+#define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_PRIVATEH__
+
+G_BEGIN_DECLS
+
+virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
+
+G_END_DECLS
+
+#endif /* __LIBVIRT_GOBJECT_DOMAIN_DEVICE_PRIVATEH__ */
diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c
b/libvirt-gobject/libvirt-gobject-domain-device.c
new file mode 100644
index 000..ae03489
--- /dev/null
+++ b/libvirt-gobject/libvirt-gobject-domain-device.c
@@ -0,0 +1,139 @@
+/*
+ * libvirt-gobject-domain-device.c: 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
+ *
+ * Author: Marc-André Lureau marcandre.lur...@redhat.com
+ */
+
+#include config.h
+
+#include libvirt/virterror.h
+#include string.h
+
+#include libvirt-glib/libvirt-glib.h
+#include libvirt-gobject/libvirt-gobject.h
+
+#include libvirt-gobject/libvirt-gobject-domain-device-private.h
+
+extern gboolean debugFlag;
+
+#define DEBUG(fmt, ...) do { if (G_UNLIKELY(debugFlag))
[libvirt] [PATCH libvirt-glib 3/6] Add GVirDomainDisk
---
libvirt-gobject/Makefile.am |2 +
libvirt-gobject/libvirt-gobject-domain-disk.c | 208 +
libvirt-gobject/libvirt-gobject-domain-disk.h | 78 +
libvirt-gobject/libvirt-gobject.h |1 +
libvirt-gobject/libvirt-gobject.sym |4 +
5 files changed, 293 insertions(+), 0 deletions(-)
create mode 100644 libvirt-gobject/libvirt-gobject-domain-disk.c
create mode 100644 libvirt-gobject/libvirt-gobject-domain-disk.h
diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am
index 8b59109..101007e 100644
--- a/libvirt-gobject/Makefile.am
+++ b/libvirt-gobject/Makefile.am
@@ -8,6 +8,7 @@ GOBJECT_HEADER_FILES = \
libvirt-gobject-main.h \
libvirt-gobject-domain-snapshot.h \
libvirt-gobject-domain-device.h \
+ libvirt-gobject-domain-disk.h \
libvirt-gobject-domain-interface.h \
libvirt-gobject-domain.h \
libvirt-gobject-interface.h \
@@ -24,6 +25,7 @@ GOBJECT_SOURCE_FILES = \
libvirt-gobject-main.c \
libvirt-gobject-domain-snapshot.c \
libvirt-gobject-domain-device.c \
+ libvirt-gobject-domain-disk.c \
libvirt-gobject-domain-interface.c \
libvirt-gobject-domain.c \
libvirt-gobject-interface.c \
diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c
b/libvirt-gobject/libvirt-gobject-domain-disk.c
new file mode 100644
index 000..fcb2596
--- /dev/null
+++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
@@ -0,0 +1,208 @@
+/*
+ * libvirt-gobject-domain-disk.c: 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
+ *
+ * Author: Marc-André Lureau marcandre.lur...@redhat.com
+ */
+
+#include config.h
+
+#include libvirt/virterror.h
+#include string.h
+
+#include libvirt-glib/libvirt-glib.h
+#include libvirt-gobject/libvirt-gobject.h
+
+#include libvirt-gobject/libvirt-gobject-domain-device-private.h
+
+extern gboolean debugFlag;
+
+#define DEBUG(fmt, ...) do { if (G_UNLIKELY(debugFlag)) g_debug(fmt, ##
__VA_ARGS__); } while (0)
+
+#define GVIR_DOMAIN_DISK_GET_PRIVATE(obj) \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK,
GVirDomainDiskPrivate))
+
+struct _GVirDomainDiskPrivate
+{
+gchar *path;
+};
+
+G_DEFINE_TYPE(GVirDomainDisk, gvir_domain_disk, GVIR_TYPE_DOMAIN_DEVICE);
+
+enum {
+PROP_0,
+PROP_PATH,
+};
+
+#define GVIR_DOMAIN_DISK_ERROR gvir_domain_disk_error_quark()
+
+
+static GQuark
+gvir_domain_disk_error_quark(void)
+{
+return g_quark_from_static_string(gvir-domain-disk);
+}
+
+static void gvir_domain_disk_get_property(GObject *object,
+ guint prop_id,
+ GValue *value,
+ GParamSpec *pspec)
+{
+GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
+GVirDomainDiskPrivate *priv = self-priv;
+
+switch (prop_id) {
+case PROP_PATH:
+g_value_set_string(value, priv-path);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+
+static void gvir_domain_disk_set_property(GObject *object,
+ guint prop_id,
+ const GValue *value,
+ GParamSpec *pspec)
+{
+GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
+GVirDomainDiskPrivate *priv = self-priv;
+
+switch (prop_id) {
+case PROP_PATH:
+g_free(priv-path);
+priv-path = g_value_dup_string(value);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+
+static void gvir_domain_disk_finalize(GObject *object)
+{
+GVirDomainDisk *self = GVIR_DOMAIN_DISK(object);
+GVirDomainDiskPrivate *priv = self-priv;
+
+DEBUG(Finalize GVirDomainDisk=%p, self);
+
+g_free(priv-path);
+
+
[libvirt] [PATCH libvirt-glib 5/6] Use G_DEFINE_BOXED_TYPE for boxed types
---
libvirt-gobject/libvirt-gobject-connection.c | 26 ++---
libvirt-gobject/libvirt-gobject-domain-disk.c | 15 +--
libvirt-gobject/libvirt-gobject-domain-interface.c | 15 +--
libvirt-gobject/libvirt-gobject-domain-snapshot.c | 25 +---
libvirt-gobject/libvirt-gobject-domain.c | 40 +++-
libvirt-gobject/libvirt-gobject-interface.c| 24 +---
libvirt-gobject/libvirt-gobject-network-filter.c | 25 +---
libvirt-gobject/libvirt-gobject-network.c | 25 +---
libvirt-gobject/libvirt-gobject-node-device.c | 25 +---
libvirt-gobject/libvirt-gobject-secret.c | 25 +---
libvirt-gobject/libvirt-gobject-storage-pool.c | 25 +---
libvirt-gobject/libvirt-gobject-storage-vol.c | 25 +---
libvirt-gobject/libvirt-gobject-stream.c | 25 +---
13 files changed, 130 insertions(+), 190 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-connection.c
b/libvirt-gobject/libvirt-gobject-connection.c
index ec3f7c3..0f6e530 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -28,6 +28,7 @@
#include libvirt-glib/libvirt-glib.h
#include libvirt-gobject/libvirt-gobject.h
+#include libvirt-gobject-compat.h
extern gboolean debugFlag;
@@ -1121,27 +1122,24 @@ GVirStoragePool
*gvir_connection_find_storage_pool_by_name(GVirConnection *conn,
return NULL;
}
-static gpointer
-gvir_connection_handle_copy(gpointer src)
+typedef struct virConnect GVirConnectionHandle;
+
+static GVirConnectionHandle*
+gvir_connection_handle_copy(GVirConnectionHandle *src)
{
-virConnectRef(src);
+virConnectRef((virConnectPtr)src);
return src;
}
-
-GType gvir_connection_handle_get_type(void)
+static void
+gvir_connection_handle_free(GVirConnectionHandle *src)
{
-static GType handle_type = 0;
-
-if (G_UNLIKELY(handle_type == 0))
-handle_type = g_boxed_type_register_static
-(GVirConnectionHandle,
- gvir_connection_handle_copy,
- (GBoxedFreeFunc)virConnectClose);
-
-return handle_type;
+virConnectClose((virConnectPtr)src);
}
+G_DEFINE_BOXED_TYPE(GVirConnectionHandle, gvir_connection_handle,
+gvir_connection_handle_copy, gvir_connection_handle_free)
+
/**
* gvir_connection_get_stream:
* @flags: flags to use for the stream
diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c
b/libvirt-gobject/libvirt-gobject-domain-disk.c
index fcb2596..b8f9dbb 100644
--- a/libvirt-gobject/libvirt-gobject-domain-disk.c
+++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
@@ -27,6 +27,7 @@
#include libvirt-glib/libvirt-glib.h
#include libvirt-gobject/libvirt-gobject.h
+#include libvirt-gobject-compat.h
#include libvirt-gobject/libvirt-gobject-domain-device-private.h
@@ -151,18 +152,8 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats)
}
-GType gvir_domain_disk_stats_get_type(void)
-{
-static GType stats_type = 0;
-
-if (G_UNLIKELY(stats_type == 0))
-stats_type = g_boxed_type_register_static
-(GVirDomainDiskStats,
- (GBoxedCopyFunc)gvir_domain_disk_stats_copy,
- (GBoxedFreeFunc)gvir_domain_disk_stats_free);
-
-return stats_type;
-}
+G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats,
+gvir_domain_disk_stats_copy, gvir_domain_disk_stats_free)
/**
* gvir_domain_disk_get_stats:
diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.c
b/libvirt-gobject/libvirt-gobject-domain-interface.c
index 83f24c7..a6720c8 100644
--- a/libvirt-gobject/libvirt-gobject-domain-interface.c
+++ b/libvirt-gobject/libvirt-gobject-domain-interface.c
@@ -27,6 +27,7 @@
#include libvirt-glib/libvirt-glib.h
#include libvirt-gobject/libvirt-gobject.h
+#include libvirt-gobject-compat.h
#include libvirt-gobject/libvirt-gobject-domain-device-private.h
@@ -151,18 +152,8 @@ gvir_domain_interface_stats_free(GVirDomainInterfaceStats
*stats)
}
-GType gvir_domain_interface_stats_get_type(void)
-{
-static GType stats_type = 0;
-
-if (G_UNLIKELY(stats_type == 0))
-stats_type = g_boxed_type_register_static
-(GVirDomainInterfaceStats,
- (GBoxedCopyFunc)gvir_domain_interface_stats_copy,
- (GBoxedFreeFunc)gvir_domain_interface_stats_free);
-
-return stats_type;
-}
+G_DEFINE_BOXED_TYPE(GVirDomainInterfaceStats, gvir_domain_interface_stats,
+gvir_domain_interface_stats_copy,
gvir_domain_interface_stats_free)
/**
* gvir_domain_interface_get_stats:
diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
index 530907d..96be997 100644
--- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
+++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
@@ -28,6 +28,7 @@
[libvirt] [PATCH libvirt-glib 2/6] Add GVirDomainInterface
---
libvirt-gobject/Makefile.am|2 +
libvirt-gobject/libvirt-gobject-domain-interface.c | 211
libvirt-gobject/libvirt-gobject-domain-interface.h | 81
libvirt-gobject/libvirt-gobject.h |1 +
libvirt-gobject/libvirt-gobject.sym|4 +
5 files changed, 299 insertions(+), 0 deletions(-)
create mode 100644 libvirt-gobject/libvirt-gobject-domain-interface.c
create mode 100644 libvirt-gobject/libvirt-gobject-domain-interface.h
diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am
index 56a047e..8b59109 100644
--- a/libvirt-gobject/Makefile.am
+++ b/libvirt-gobject/Makefile.am
@@ -8,6 +8,7 @@ GOBJECT_HEADER_FILES = \
libvirt-gobject-main.h \
libvirt-gobject-domain-snapshot.h \
libvirt-gobject-domain-device.h \
+ libvirt-gobject-domain-interface.h \
libvirt-gobject-domain.h \
libvirt-gobject-interface.h \
libvirt-gobject-network.h \
@@ -23,6 +24,7 @@ GOBJECT_SOURCE_FILES = \
libvirt-gobject-main.c \
libvirt-gobject-domain-snapshot.c \
libvirt-gobject-domain-device.c \
+ libvirt-gobject-domain-interface.c \
libvirt-gobject-domain.c \
libvirt-gobject-interface.c \
libvirt-gobject-network.c \
diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.c
b/libvirt-gobject/libvirt-gobject-domain-interface.c
new file mode 100644
index 000..83f24c7
--- /dev/null
+++ b/libvirt-gobject/libvirt-gobject-domain-interface.c
@@ -0,0 +1,211 @@
+/*
+ * libvirt-gobject-domain-interface.c: 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
+ *
+ * Author: Marc-André Lureau marcandre.lur...@redhat.com
+ */
+
+#include config.h
+
+#include libvirt/virterror.h
+#include string.h
+
+#include libvirt-glib/libvirt-glib.h
+#include libvirt-gobject/libvirt-gobject.h
+
+#include libvirt-gobject/libvirt-gobject-domain-device-private.h
+
+extern gboolean debugFlag;
+
+#define DEBUG(fmt, ...) do { if (G_UNLIKELY(debugFlag)) g_debug(fmt, ##
__VA_ARGS__); } while (0)
+
+#define GVIR_DOMAIN_INTERFACE_GET_PRIVATE(obj) \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_INTERFACE,
GVirDomainInterfacePrivate))
+
+struct _GVirDomainInterfacePrivate
+{
+gchar *path;
+};
+
+G_DEFINE_TYPE(GVirDomainInterface, gvir_domain_interface,
GVIR_TYPE_DOMAIN_DEVICE);
+
+enum {
+PROP_0,
+PROP_PATH,
+};
+
+#define GVIR_DOMAIN_INTERFACE_ERROR gvir_domain_interface_error_quark()
+
+
+static GQuark
+gvir_domain_interface_error_quark(void)
+{
+return g_quark_from_static_string(gvir-domain-interface);
+}
+
+static void gvir_domain_interface_get_property(GObject *object,
+ guint prop_id,
+ GValue *value,
+ GParamSpec *pspec)
+{
+GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object);
+GVirDomainInterfacePrivate *priv = self-priv;
+
+switch (prop_id) {
+case PROP_PATH:
+g_value_set_string(value, priv-path);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+
+static void gvir_domain_interface_set_property(GObject *object,
+ guint prop_id,
+ const GValue *value,
+ GParamSpec *pspec)
+{
+GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object);
+GVirDomainInterfacePrivate *priv = self-priv;
+
+switch (prop_id) {
+case PROP_PATH:
+g_free(priv-path);
+priv-path = g_value_dup_string(value);
+break;
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+
+static void gvir_domain_interface_finalize(GObject *object)
+{
+GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object);
+
[libvirt] [PATCH libvirt-glib 4/6] Copy G_DEFINE_BOXED_TYPE from GObject 2.26+
---
libvirt-gobject/Makefile.am |1 +
libvirt-gobject/libvirt-gobject-compat.h | 72 ++
2 files changed, 73 insertions(+), 0 deletions(-)
create mode 100644 libvirt-gobject/libvirt-gobject-compat.h
diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am
index 101007e..d975701 100644
--- a/libvirt-gobject/Makefile.am
+++ b/libvirt-gobject/Makefile.am
@@ -48,6 +48,7 @@ libvirt_gobject_1_0_la_HEADERS = \
$(GOBJECT_HEADER_FILES) \
libvirt-gobject-input-stream.h
nodist_libvirt_gobject_1_0_la_HEADERS = \
+ libvirt-gobject-compat.h \
libvirt-gobject-domain-device-private.h \
libvirt-gobject-enums.h
libvirt_gobject_1_0_la_SOURCES = \
diff --git a/libvirt-gobject/libvirt-gobject-compat.h
b/libvirt-gobject/libvirt-gobject-compat.h
new file mode 100644
index 000..7b4be45
--- /dev/null
+++ b/libvirt-gobject/libvirt-gobject-compat.h
@@ -0,0 +1,72 @@
+/*
+ * libvirt-gobject-compat.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
+ *
+ * Author: Marc-André Lureau marcandre.lur...@redhat.com
+ */
+
+#ifndef __LIBVIRT_GOBJECT_COMPAT_H__
+#define __LIBVIRT_GOBJECT_COMPAT_H__
+
+#include glib-object.h
+
+#if !GLIB_CHECK_VERSION(2,26,0)
+#define G_DEFINE_BOXED_TYPE(TypeName, type_name, copy_func, free_func)
G_DEFINE_BOXED_TYPE_WITH_CODE (TypeName, type_name, copy_func, free_func, {})
+#define G_DEFINE_BOXED_TYPE_WITH_CODE(TypeName, type_name, copy_func,
free_func, _C_) _G_DEFINE_BOXED_TYPE_BEGIN (TypeName, type_name, copy_func,
free_func) {_C_;} _G_DEFINE_TYPE_EXTENDED_END()
+#if __GNUC__ 2 || (__GNUC__ == 2 __GNUC_MINOR__ = 7)
+#define _G_DEFINE_BOXED_TYPE_BEGIN(TypeName, type_name, copy_func, free_func) \
+GType \
+type_name##_get_type (void) \
+{ \
+ static volatile gsize g_define_type_id__volatile = 0; \
+ if (g_once_init_enter (g_define_type_id__volatile)) \
+{ \
+ GType (* _g_register_boxed) \
+(const gchar *, \
+ union \
+ { \
+ TypeName * (*do_copy_type) (TypeName *); \
+ TypeName * (*do_const_copy_type) (const TypeName *); \
+ GBoxedCopyFunc do_copy_boxed; \
+ } __attribute__((__transparent_union__)), \
+ union \
+ { \
+ void (* do_free_type) (TypeName *); \
+ GBoxedFreeFunc do_free_boxed; \
+ } __attribute__((__transparent_union__)) \
+) = g_boxed_type_register_static; \
+ GType g_define_type_id = \
+_g_register_boxed (g_intern_static_string (#TypeName), copy_func,
free_func); \
+ { /* custom code follows */
+#else
+#define _G_DEFINE_BOXED_TYPE_BEGIN(TypeName, type_name, copy_func, free_func) \
+GType \
+type_name##_get_type (void) \
+{ \
+ static volatile gsize g_define_type_id__volatile = 0; \
+ if (g_once_init_enter (g_define_type_id__volatile)) \
+{ \
+ GType g_define_type_id = \
+g_boxed_type_register_static (g_intern_static_string (#TypeName), \
+ (GBoxedCopyFunc) copy_func, \
+ (GBoxedFreeFunc) free_func); \
+ { /* custom code follows */
+#endif /* __GNUC__ */
+#endif /* glib 2.26 */
+
+#endif /* __LIBVIRT_GOBJECT_COMPAT_H__ */
--
1.7.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-glib 6/6] Remove useless memset(0) of private data
GObject initialization ensures objects and their private data are 0's.
---
libvirt-gconfig/libvirt-gconfig-capabilities.c|6 +-
libvirt-gconfig/libvirt-gconfig-domain-snapshot.c |6 +-
libvirt-gconfig/libvirt-gconfig-domain.c |6 +-
libvirt-gconfig/libvirt-gconfig-interface.c |6 +-
libvirt-gconfig/libvirt-gconfig-network-filter.c |6 +-
libvirt-gconfig/libvirt-gconfig-network.c |6 +-
libvirt-gconfig/libvirt-gconfig-node-device.c |6 +-
libvirt-gconfig/libvirt-gconfig-object.c |6 +-
libvirt-gconfig/libvirt-gconfig-secret.c |6 +-
libvirt-gconfig/libvirt-gconfig-storage-pool.c|6 +-
libvirt-gconfig/libvirt-gconfig-storage-vol.c |6 +-
libvirt-gobject/libvirt-gobject-connection.c |2 --
libvirt-gobject/libvirt-gobject-domain-snapshot.c |6 +-
libvirt-gobject/libvirt-gobject-domain.c |6 +-
libvirt-gobject/libvirt-gobject-interface.c |6 +-
libvirt-gobject/libvirt-gobject-manager.c |2 --
libvirt-gobject/libvirt-gobject-network-filter.c |6 +-
libvirt-gobject/libvirt-gobject-network.c |6 +-
libvirt-gobject/libvirt-gobject-node-device.c |6 +-
libvirt-gobject/libvirt-gobject-secret.c |6 +-
libvirt-gobject/libvirt-gobject-storage-pool.c|4 +---
libvirt-gobject/libvirt-gobject-storage-vol.c |6 +-
22 files changed, 20 insertions(+), 102 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities.c
b/libvirt-gconfig/libvirt-gconfig-capabilities.c
index 75edc2d..703673d 100644
--- a/libvirt-gconfig/libvirt-gconfig-capabilities.c
+++ b/libvirt-gconfig/libvirt-gconfig-capabilities.c
@@ -51,13 +51,9 @@ static void
gvir_config_capabilities_class_init(GVirConfigCapabilitiesClass *kla
static void gvir_config_capabilities_init(GVirConfigCapabilities *conn)
{
-GVirConfigCapabilitiesPrivate *priv;
-
DEBUG(Init GVirConfigCapabilities=%p, conn);
-priv = conn-priv = GVIR_CONFIG_CAPABILITIES_GET_PRIVATE(conn);
-
-memset(priv, 0, sizeof(*priv));
+conn-priv = GVIR_CONFIG_CAPABILITIES_GET_PRIVATE(conn);
}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c
b/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c
index f6151fe..970de87 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c
@@ -51,13 +51,9 @@ static void
gvir_config_domain_snapshot_class_init(GVirConfigDomainSnapshotClass
static void gvir_config_domain_snapshot_init(GVirConfigDomainSnapshot *conn)
{
-GVirConfigDomainSnapshotPrivate *priv;
-
DEBUG(Init GVirConfigDomainSnapshot=%p, conn);
-priv = conn-priv = GVIR_CONFIG_DOMAIN_SNAPSHOT_GET_PRIVATE(conn);
-
-memset(priv, 0, sizeof(*priv));
+conn-priv = GVIR_CONFIG_DOMAIN_SNAPSHOT_GET_PRIVATE(conn);
}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c
b/libvirt-gconfig/libvirt-gconfig-domain.c
index 77115ba..2d5aeb5 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain.c
@@ -134,13 +134,9 @@ static void
gvir_config_domain_class_init(GVirConfigDomainClass *klass)
static void gvir_config_domain_init(GVirConfigDomain *conn)
{
-GVirConfigDomainPrivate *priv;
-
DEBUG(Init GVirConfigDomain=%p, conn);
-priv = conn-priv = GVIR_CONFIG_DOMAIN_GET_PRIVATE(conn);
-
-memset(priv, 0, sizeof(*priv));
+conn-priv = GVIR_CONFIG_DOMAIN_GET_PRIVATE(conn);
}
diff --git a/libvirt-gconfig/libvirt-gconfig-interface.c
b/libvirt-gconfig/libvirt-gconfig-interface.c
index f3f5d39..631dc3b 100644
--- a/libvirt-gconfig/libvirt-gconfig-interface.c
+++ b/libvirt-gconfig/libvirt-gconfig-interface.c
@@ -51,13 +51,9 @@ static void
gvir_config_interface_class_init(GVirConfigInterfaceClass *klass)
static void gvir_config_interface_init(GVirConfigInterface *conn)
{
-GVirConfigInterfacePrivate *priv;
-
DEBUG(Init GVirConfigInterface=%p, conn);
-priv = conn-priv = GVIR_CONFIG_INTERFACE_GET_PRIVATE(conn);
-
-memset(priv, 0, sizeof(*priv));
+conn-priv = GVIR_CONFIG_INTERFACE_GET_PRIVATE(conn);
}
diff --git a/libvirt-gconfig/libvirt-gconfig-network-filter.c
b/libvirt-gconfig/libvirt-gconfig-network-filter.c
index f1c99df..4138fc1 100644
--- a/libvirt-gconfig/libvirt-gconfig-network-filter.c
+++ b/libvirt-gconfig/libvirt-gconfig-network-filter.c
@@ -51,13 +51,9 @@ static void
gvir_config_network_filter_class_init(GVirConfigNetworkFilterClass *
static void gvir_config_network_filter_init(GVirConfigNetworkFilter *conn)
{
-GVirConfigNetworkFilterPrivate *priv;
-
DEBUG(Init GVirConfigNetworkFilter=%p, conn);
-priv = conn-priv = GVIR_CONFIG_NETWORK_FILTER_GET_PRIVATE(conn);
-
-memset(priv, 0, sizeof(*priv));
+conn-priv = GVIR_CONFIG_NETWORK_FILTER_GET_PRIVATE(conn);
}
diff --git
Re: [libvirt] [Qemu-devel] qemu and qemu.git - Migration + disk stress introduces qcow2 corruptions
On 11/14/2011 04:16 AM, Daniel P. Berrange wrote: On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: On 11/11/2011 12:15 PM, Kevin Wolf wrote: Am 10.11.2011 22:30, schrieb Anthony Liguori: Live migration with qcow2 or any other image format is just not going to work right now even with proper clustered storage. I think doing a block level flush cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case. If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between? What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Delayed open isn't a panacea. With the series I sent, we should be able to migration with a qcow2 file on coherent shared storage. There are two other cases that we care about: migration with nfs cache!=none and direct attached storage with cache!=none Whether the open is deferred matters less with NFS than if the open happens after the close on the source. To fix NFS cache!=none, we would have to do a bdrv_close() before sending the last byte of migration data and make sure that we bdrv_open() after receiving the last byte of migration data. The problem with this IMHO is it creates a large window where noone has the file open and you're critically vulnerable to losing your VM. I'm much more in favor of a smarter caching policy. If we can fcntl() our way to O_DIRECT on NFS, that would be fairly interesting. I'm not sure if this is supported today but it's something we could look into adding in the kernel. That way we could force NFS to O_DIRECT during migration which would solve this problem robustly. Deferred open doesn't help with direct attached storage. There simple is no guarantee that there isn't data in the page cache. Again, I think defaulting DAS to cache=none|directsync is what makes the most sense here. We can even add a migration blocker for DAS with cache=on. If we can do dynamic toggling of the cache setting, then that's pretty friendly at the end of the day. Regards, Anthony Liguori Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v2 0/4] Libvirt for PowerPC
Hi, Recent development in KVM for 64-bit Power ISA Book3S machines, allows users to run multiple KVM guest instances on POWER7 and PPC970 processor based systems. Also qemu-system-ppc64 has been enhanced to support a new machine type pseries suitable for Power Book3S machines. This addition effectively brings the KVM+qemu combination to run multiple guest instances on a Power Book3S machine. Libvirt continues to be the key interface to configure and manage the KVM guest instances on x86. This patch set is an effort to enable libvirt to support KVM guest configuration and management on Power Book3S machines. Based on community discussion around the earlier version, this patch series augments the present 'kvm' driver to support PowerPC-KVM based guests. Since some of the supported devices vary between architectures, the code is now reworked to dynamically choose the correct handler function based on guest domain architecture at runtime (def-os.arch). This patch series consists of 4 patches : Patch 1/4 : Use sysfs to gather host topology. Presently libvirt depends on /proc/cpuinfo gather CPU, cores, threads, etc This is highly architecture dependent. A alternative is to use sysfs, which provides a platform-neutral interface to parse CPU topology. Patch 2/4 : Add PowerPC CPU Driver Patch 3/4 : Add support for qemu-system-ppc64 Patch 4/4 : Refactor qemu commmand-line generation to separate out arch- specific options from generic command line. Changelog from v1: * Patches 1,2,3 unchanged ; The hacks in Patch 4 of v1 have been replaced by a new patch to neatly select arch-specific features. Awaiting comments and feedback, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v2 2/4] PowerPC : Add PPC CPU Driver
From 918e49581c91c007086a0a7189c9b6cf83ba2bb0 Mon Sep 17 00:00:00 2001
From: Prerna Saxena pre...@linux.vnet.ibm.com
Date: Mon, 3 Oct 2011 05:56:20 -0700
Subject: [PATCH 2/4] Add PPC cpu driver.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Anton Blanchard an...@au.ibm.com
---
src/Makefile.am |3 +-
src/cpu/cpu.c |2 +
src/cpu/cpu_powerpc.c | 81 +
src/cpu/cpu_powerpc.h | 32 +++
4 files changed, 117 insertions(+), 1 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index e931d41..2c4e6ba 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -524,7 +524,8 @@ CPU_SOURCES =
\
cpu/cpu.h cpu/cpu.c \
cpu/cpu_generic.h cpu/cpu_generic.c \
cpu/cpu_x86.h cpu/cpu_x86.c cpu/cpu_x86_data.h \
- cpu/cpu_map.h cpu/cpu_map.c
+ cpu/cpu_map.h cpu/cpu_map.c cpu/cpu_powerpc.h \
+ cpu/cpu_powerpc.c
VMX_SOURCES = \
vmx/vmx.c vmx/vmx.h
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index b919b6e..e4149e2 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -28,6 +28,7 @@
#include xml.h
#include cpu.h
#include cpu_x86.h
+#include cpu_powerpc.h
#include cpu_generic.h
@@ -36,6 +37,7 @@
static struct cpuArchDriver *drivers[] = {
cpuDriverX86,
+cpuDriverPowerPC,
/* generic driver must always be the last one */
cpuDriverGeneric
};
diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
new file mode 100644
index 000..6ceedc3
--- /dev/null
+++ b/src/cpu/cpu_powerpc.c
@@ -0,0 +1,81 @@
+/*
+ * cpu_powerpc.h: CPU driver for PowerPC CPUs
+ *
+ * Copyright (C) Copyright (C) IBM Corporation, 2010
+ *
+ * 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:
+ * Anton Blanchard an...@au.ibm.com
+ * Prerna Saxena pre...@linux.vnet.ibm.com
+ */
+
+#include config.h
+
+#include memory.h
+#include cpu.h
+
+
+#define VIR_FROM_THIS VIR_FROM_CPU
+
+static const char *archs[] = { ppc64 };
+
+static union cpuData *
+PowerPCNodeData(void)
+{
+union cpuData *data;
+
+if (VIR_ALLOC(data) 0) {
+virReportOOMError();
+return NULL;
+}
+
+return data;
+}
+
+
+static int
+PowerPCDecode(virCPUDefPtr cpu,
+ const union cpuData *data,
+ const char **models,
+ unsigned int nmodels,
+ const char *preferred)
+{
+ return 0;
+}
+
+static int
+PowerPCDataFree(union cpuData *data)
+{
+ if (data == NULL)
+ return 0;
+
+ VIR_FREE(data);
+}
+
+struct cpuArchDriver cpuDriverPowerPC = {
+.name = ppc64,
+.arch = archs,
+.narch = ARRAY_CARDINALITY(archs),
+.compare= NULL,
+.decode = PowerPCDecode,
+.encode = NULL,
+.free = PowerPCDataFree,
+.nodeData = PowerPCNodeData,
+.guestData = NULL,
+.baseline = NULL,
+.update = NULL,
+.hasFeature = NULL,
+};
diff --git a/src/cpu/cpu_powerpc.h b/src/cpu/cpu_powerpc.h
new file mode 100644
index 000..2e0c1a5
--- /dev/null
+++ b/src/cpu/cpu_powerpc.h
@@ -0,0 +1,32 @@
+/*
+ * cpu_powerpc.h: CPU driver for PowerPC CPUs
+ *
+ * Copyright (C) Copyright (C) IBM Corporation, 2010
+ *
+ * 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:
+ * Anton Blanchard an...@au.ibm.com
+ * Prerna Saxena pre...@linux.vnet.ibm.com
+ */
+
+#ifndef
[libvirt] [RFC PATCH v2 1/4] PowerPC : Use Sysfs to gather host topology
From 9084802bdf7a2c10d6a913cb7dde001ab6ab3543 Mon Sep 17 00:00:00 2001
From: Prerna Saxena pre...@linux.vnet.ibm.com
Date: Mon, 3 Oct 2011 05:45:30 -0700
Subject: [PATCH 1/4] Use sysfs to gather host topology, in place of
/proc/cpuinfo
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
src/nodeinfo.c | 106 +++-
1 files changed, 36 insertions(+), 70 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 6448b79..cdd339d 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -30,6 +30,7 @@
#include errno.h
#include dirent.h
#include sys/utsname.h
+#include sched.h
#if HAVE_NUMACTL
# define NUMA_VERSION1_COMPATIBILITY 1
@@ -191,6 +192,11 @@ static int parse_socket(unsigned int cpu)
return ret;
}
+static int parse_core(unsigned int cpu)
+{
+return get_cpu_value(cpu, topology/core_id, false);
+}
+
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
virNodeInfoPtr nodeinfo,
bool need_hyperthreads)
@@ -199,15 +205,14 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
DIR *cpudir = NULL;
struct dirent *cpudirent = NULL;
unsigned int cpu;
-unsigned long cur_threads;
-int socket;
-unsigned long long socket_mask = 0;
-unsigned int remaining;
+unsigned long core, socket, cur_threads;
+cpu_set_t core_mask;
+cpu_set_t socket_mask;
int online;
nodeinfo-cpus = 0;
nodeinfo-mhz = 0;
-nodeinfo-cores = 1;
+nodeinfo-cores = 0;
nodeinfo-nodes = 1;
# if HAVE_NUMACTL
@@ -221,20 +226,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* NOTE: hyperthreads are ignored here; they are parsed out of /sys */
while (fgets(line, sizeof(line), cpuinfo) != NULL) {
char *buf = line;
-if (STRPREFIX(buf, processor)) { /* aka a single logical CPU */
-buf += 9;
-while (*buf c_isspace(*buf))
-buf++;
-if (*buf != ':') {
-nodeReportError(VIR_ERR_INTERNAL_ERROR,
-%s, _(parsing cpuinfo processor));
-return -1;
-}
-nodeinfo-cpus++;
# if defined(__x86_64__) || \
defined(__amd64__) || \
defined(__i386__)
-} else if (STRPREFIX(buf, cpu MHz)) {
+if (STRPREFIX(buf, cpu MHz)) {
char *p;
unsigned int ui;
buf += 9;
@@ -249,24 +244,9 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
(*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo-mhz = ui;
-} else if (STRPREFIX(buf, cpu cores)) { /* aka cores */
-char *p;
-unsigned int id;
-buf += 9;
-while (*buf c_isspace(*buf))
-buf++;
-if (*buf != ':' || !buf[1]) {
-nodeReportError(VIR_ERR_INTERNAL_ERROR,
-_(parsing cpuinfo cpu cores %c), *buf);
-return -1;
-}
-if (virStrToLong_ui(buf+1, p, 10, id) == 0
- (*p == '\0' || c_isspace(*p))
- id nodeinfo-cores)
-nodeinfo-cores = id;
# elif defined(__powerpc__) || \
defined(__powerpc64__)
-} else if (STRPREFIX(buf, clock)) {
+if (STRPREFIX(buf, clock)) {
char *p;
unsigned int ui;
buf += 5;
@@ -281,53 +261,30 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
(*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo-mhz = ui;
-# elif defined(__s390__) || \
-defined(__s390x__)
-} else if (STRPREFIX(buf, # processors)) {
-char *p;
-unsigned int ui;
-buf += 12;
-while (*buf c_isspace(*buf))
-buf++;
-if (*buf != ':' || !buf[1]) {
-nodeReportError(VIR_ERR_INTERNAL_ERROR,
-_(parsing number of processors %c), *buf);
-return -1;
-}
-if (virStrToLong_ui(buf+1, p, 10, ui) == 0
- (*p == '\0' || c_isspace(*p)))
-nodeinfo-cpus = ui;
/* No other interesting infos are available in /proc/cpuinfo.
* However, there is a line identifying processor's version,
* identification and machine, but we don't want it to be caught
* and parsed in next iteration, because it is not in expected
* format and thus lead to error. */
-break;
# else
# warning Parser for /proc/cpuinfo needs to be adapted for your architecture
# endif
}
}
-if (!nodeinfo-cpus) {
-nodeReportError(VIR_ERR_INTERNAL_ERROR,
-%s, _(no cpus found));
-return -1;
-}
-
-if
[libvirt] [RFC PATCH v2 3/4] PowerPC : Add support for ppc64 qemu
From cf5850d807fb6d86a16879a3e4fe120a96b78e27 Mon Sep 17 00:00:00 2001
From: Prerna Saxena pre...@linux.vnet.ibm.com
Date: Mon, 3 Oct 2011 06:01:33 -0700
Subject: [PATCH 3/4] Add support for ppc64 qemu
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
src/qemu/qemu_capabilities.c | 64 ++
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 26a7f11..f9a465d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -185,6 +185,7 @@ static const struct qemu_arch_info const arch_info_hvm[] = {
{ mipsel, 32, NULL, qemu-system-mipsel, NULL, NULL, 0 },
{ sparc, 32, NULL, qemu-system-sparc, NULL, NULL, 0 },
{ ppc,32, NULL, qemu-system-ppc,NULL, NULL, 0 },
+{ ppc64,64, NULL, qemu-system-ppc64,NULL, NULL, 0 },
{ itanium, 64, NULL, qemu-system-ia64, NULL, NULL, 0 },
{ s390x, 64, NULL, qemu-system-s390x, NULL, NULL, 0 },
};
@@ -477,6 +478,67 @@ error:
return -1;
}
+/* ppc64 parser.
+ * Format : PowerPC machine description
+ */
+static int
+qemuCapsParsePPCModels(const char *output,
+ unsigned int *retcount,
+ const char ***retcpus)
+{
+const char *p = output;
+const char *next;
+unsigned int count = 0;
+const char **cpus = NULL;
+int i;
+do {
+const char *t;
+
+if ((next = strchr(p, '\n')))
+next++;
+
+if (!STRPREFIX(p, PowerPC ))
+continue;
+
+/* Skip the preceding sub-string PowerPC */
+p += 8;
+
+/*Malformed string, does not obey the format 'PowerPC model desc'*/
+if (!(t = strchr(p, ' ')) || (next t = next))
+continue;
+
+if (*p == '\0' || *p == '\n')
+continue;
+
+if (retcpus) {
+unsigned int len;
+
+if (VIR_REALLOC_N(cpus, count + 1) 0)
+goto error;
+
+if (t)
+len = t - p - 1;
+
+if (!(cpus[count] = strndup(p, len)))
+goto error;
+}
+count++;
+} while ((p = next));
+
+if (retcount)
+*retcount = count;
+if (retcpus)
+*retcpus = cpus;
+return 0;
+
+error:
+if (cpus) {
+for (i = 0; i count; i++)
+VIR_FREE(cpus[i]);
+}
+VIR_FREE(cpus);
+return -1;
+}
int
qemuCapsProbeCPUModels(const char *qemu,
@@ -497,6 +559,8 @@ qemuCapsProbeCPUModels(const char *qemu,
if (STREQ(arch, i686) || STREQ(arch, x86_64))
parse = qemuCapsParseX86Models;
+else if (STREQ(arch, ppc64))
+parse = qemuCapsParsePPCModels;
else {
VIR_DEBUG(don't know how to parse %s CPU models, arch);
return 0;
--
1.7.7
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v2 4/4] PowerPC : Bifurcate arch-specific qemu initialization code from generic qemu command line generation.
From ae7764be4454be1900fc3ee0a03bf819d5cd12de Mon Sep 17 00:00:00 2001
From: Prerna Saxena pre...@linux.vnet.ibm.com
Date: Mon, 14 Nov 2011 19:43:26 +0530
Subject: [PATCH 4/4] Separate out arch-specific qemu initialization from
generic qemu commandline building code.
At present, qemuBuildCommandLine emits many x86-specific options while
generating the qemu command line. This patch proposes a framework to add
arch-specific handler functions for generating different aspects of
command line.
At present, it is used to prevent x86-specific qemu options from
cluttering the qemu command line for other architectures. Going forward,
if newer drivers get defined for any arch, the code for handling these
might be added by extending the handler function for that architecture.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
src/qemu/qemu_command.c | 220 ---
src/qemu/qemu_command.h | 20
2 files changed, 169 insertions(+), 71 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b044050..61dabbf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -107,6 +107,13 @@ VIR_ENUM_IMPL(qemuDomainFSDriver,
VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
local,
handle);
+virQemuCommandLineFunction qemuCmdLineX86 =
+{ .qemuBuildArchFunction = qemuBuildX86CommandLine,
+};
+
+virQemuCommandLineFunction qemuCmdLinePPC64 =
+{ .qemuBuildArchFunction = NULL,
+};
static void
uname_normalize (struct utsname *ut)
@@ -3254,6 +3261,8 @@ qemuBuildCommandLine(virConnectPtr conn,
bool emitBootindex = false;
int usbcontroller = 0;
bool usblegacy = false;
+char *command_str;
+virQemuCommandLineFunctionPtr qemuCmdFunc;
uname_normalize(ut);
virUUIDFormat(def-uuid, uuid);
@@ -3409,54 +3418,22 @@ qemuBuildCommandLine(virConnectPtr conn,
}
}
-if ((def-os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE)
-(def-os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) {
-virSysinfoDefPtr source = NULL;
-bool skip_uuid = false;
-
-if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SMBIOS_TYPE)) {
-qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-_(the QEMU binary %s does not support smbios settings),
-emulator);
-goto error;
-}
-
-/* should we really error out or just warn in those cases ? */
-if (def-os.smbios_mode == VIR_DOMAIN_SMBIOS_HOST) {
-if (driver-hostsysinfo == NULL) {
-qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-_(Host SMBIOS information is not available));
-goto error;
-}
-source = driver-hostsysinfo;
-/* Host and guest uuid must differ, by definition of UUID. */
-skip_uuid = true;
-} else if (def-os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO) {
-if (def-sysinfo == NULL) {
-qemuReportError(VIR_ERR_XML_ERROR,
-_(Domain '%s' sysinfo are not available),
- def-name);
-goto error;
-}
-source = def-sysinfo;
-/* domain_conf guaranteed that system_uuid matches guest uuid. */
-}
-if (source != NULL) {
-char *smbioscmd;
+if (!strcmp(def-os.arch, i686) ||
+ !strcmp(def-os.arch, x86_64))
+qemuCmdFunc = qemuCmdLineX86;
+else {
+if (!strcmp(def-os.arch, ppc64))
+qemuCmdFunc = qemuCmdLinePPC64;
+ }
-smbioscmd = qemuBuildSmbiosBiosStr(source);
-if (smbioscmd != NULL) {
-virCommandAddArgList(cmd, -smbios, smbioscmd, NULL);
-VIR_FREE(smbioscmd);
-}
-smbioscmd = qemuBuildSmbiosSystemStr(source, skip_uuid);
-if (smbioscmd != NULL) {
-virCommandAddArgList(cmd, -smbios, smbioscmd, NULL);
-VIR_FREE(smbioscmd);
-}
-}
+if (qemuCmdFunc-qemuBuildArchFunction) {
+command_str = (qemuCmdFunc-qemuBuildArchFunction)(conn, driver, def,
+qemuCaps);
+if (command_str) {
+virCommandAddArg(cmd, command_str);
+VIR_FREE(command_str);
+ }
}
-
/*
* NB, -nographic *MUST* come before any serial, or monitor
* or parallel port flags due to QEMU craziness, where it
@@ -3475,25 +3452,6 @@ qemuBuildCommandLine(virConnectPtr conn,
-nodefaults); /* Disable default guest devices */
}
-/* Serial graphics adapter */
-if (def-os.bios.useserial == VIR_DOMAIN_BIOS_USESERIAL_YES) {
-if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
-_(qemu does not support -device));
[libvirt] [PATCH TECHPREVIEW RFC 0/4] LibSSH2 transport option for libvirt
This patchset adds a new secure transport layer to enable secure connections without the need to spawn external processes. The layer is implemented into virNetSocket. This patches still lack some functionality, which I will add before the final version is done, but I'd like to have some feedback about the features and mistakes I made, so I can incorporate them into the final version. Unfortunately, while working with libssh2, I found several bugs, and older versions (1.2.2, which is default in gentoo) are unusable (same error codes on authentication failure and closed socket and others). I decided to limit version of libssh2 to 1.3.0 for the transport to work. Please let me know if you find something that would you like to see implemented or improoved or I've done wrong, or you've got some questions. Usage: virsh -c qemu+libssh://root@locahost:22/system (user/port/host may be omitted, these are defaults) Possible problems: - segmentation fault in libssh2 when corrupted known_hosts file is used (will investigate) Things that have to be done: - private key authentication - SSH agent support - Add unknown host key dialog - documentation - change error codes and messages (any suggestions?) Thanks for your time Peter Peter Krempa (4): libssh2_transport: add main libssh2 transport implementation libssh2_transport: add ssh context support to virNetSocket libssh2_transport: Add libssh2 session support to net client code libssh2_transport: Use libssh2 driver code in remote driver configure.ac | 40 ++- po/POTFILES.in|1 + src/Makefile.am |9 + src/libvirt_private.syms |1 + src/remote/remote_driver.c| 97 -- src/rpc/virnetclient.c| 66 +++ src/rpc/virnetclient.h| 11 + src/rpc/virnetlibsshcontext.c | 900 + src/rpc/virnetlibsshcontext.h | 76 src/rpc/virnetsocket.c| 137 +++- src/rpc/virnetsocket.h| 12 + 11 files changed, 1318 insertions(+), 32 deletions(-) create mode 100644 src/rpc/virnetlibsshcontext.c create mode 100644 src/rpc/virnetlibsshcontext.h -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH TECHPREVIEW RFC 4/4] libssh2_transport: Use libssh2 driver code in remote driver
This patch adds URI options to support libssh2 transport in the remote
driver.
A new transport sceme is introduced eg. qemu+libssh://... that
utilizes the libssh2 code added in previous patches.
The libssh2 code requires the authentication callback to be able to
perform keyboard-interactive authentication or to ask t passprhases or
add host keys to known hosts database.
Added URI components:
- known_hosts - path to a knownHosts file in OpenSSH format to check
for known ssh host keys
- no_verify - this old option is abused to indicate desired behavior,
what to do while checking host keys:
options: - normal: behave as ssh, ask to add a new key,
reject unknown
- auto_add: add new keys automaticaly, reject
unknown
- ignore: don't check host key.
*src/remote/remote_driver.c: -Clean up whitespace between function name
and parentheses.
- Add libssh2 transport scheme
- Add URI components to configure libssh2
transport
TODO:
- Add documentation to web-page documents and man pages regarding new
URI options
---
src/remote/remote_driver.c | 97 +++
1 files changed, 70 insertions(+), 27 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 94fd3e7..652904c 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -303,12 +303,14 @@ enum virDrvOpenRemoteFlags {
* - xxx+tcp:///- TCP connection to localhost
* - xxx+unix:/// - UNIX domain socket
* - xxx:///- UNIX domain socket
+ * - xxx+ssh:///- SSH connection (legacy)
+ * - xxx+libssh:/// - SSH connection (using libssh2)
*/
static int
-doRemoteOpen (virConnectPtr conn,
- struct private_data *priv,
- virConnectAuthPtr auth ATTRIBUTE_UNUSED,
- unsigned int flags)
+doRemoteOpen(virConnectPtr conn,
+ struct private_data *priv,
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+ unsigned int flags)
{
struct qparam_set *vars = NULL;
char *transport_str = NULL;
@@ -316,6 +318,7 @@ doRemoteOpen (virConnectPtr conn,
trans_tls,
trans_unix,
trans_ssh,
+trans_libssh,
trans_ext,
trans_tcp,
} transport;
@@ -341,9 +344,9 @@ doRemoteOpen (virConnectPtr conn,
else
transport = trans_unix;
} else {
-if (STRCASEEQ (transport_str, tls))
+if (STRCASEEQ(transport_str, tls))
transport = trans_tls;
-else if (STRCASEEQ (transport_str, unix)) {
+else if (STRCASEEQ(transport_str, unix)) {
if (conn-uri-server) {
remoteError(VIR_ERR_INVALID_ARG,
_(using unix socket and remote
@@ -353,16 +356,18 @@ doRemoteOpen (virConnectPtr conn,
} else {
transport = trans_unix;
}
-} else if (STRCASEEQ (transport_str, ssh))
+} else if (STRCASEEQ(transport_str, ssh))
transport = trans_ssh;
-else if (STRCASEEQ (transport_str, ext))
+else if (STRCASEEQ(transport_str, libssh))
+transport = trans_libssh;
+else if (STRCASEEQ(transport_str, ext))
transport = trans_ext;
-else if (STRCASEEQ (transport_str, tcp))
+else if (STRCASEEQ(transport_str, tcp))
transport = trans_tcp;
else {
remoteError(VIR_ERR_INVALID_ARG, %s,
_(remote_open: transport in URL not
recognised
- (should be tls|unix|ssh|ext|tcp)));
+ (should be tls|unix|ssh|ext|tcp|libssh)));
return VIR_DRV_OPEN_ERROR;
}
}
@@ -380,6 +385,8 @@ doRemoteOpen (virConnectPtr conn,
bool sanity = true, verify = true, tty ATTRIBUTE_UNUSED = true;
char *pkipath = NULL, *keyfile = NULL;
+char *knownHostsVerify = NULL, *knownHosts = NULL;
+
/* Return code from this function, and the private data. */
int retcode = VIR_DRV_OPEN_ERROR;
@@ -426,49 +433,57 @@ doRemoteOpen (virConnectPtr conn,
for (i = 0; i vars-n; i++) {
var = vars-p[i];
-if (STRCASEEQ (var-name, name)) {
+if (STRCASEEQ(var-name, name)) {
VIR_FREE(name);
-name = strdup (var-value);
+name = strdup(var-value);
if (!name) goto out_of_memory;
[libvirt] [PATCH TECHPREVIEW RFC 1/4] libssh2_transport: add main libssh2 transport implementation
This patch adds helper functions to libssh2 that enable us to use libssh2 in conjunction with libvirt-native virNetSocket-s instead of using a spawned ssh client process. This implemetation supports tunneled plaintext, keyboard-interactive, private key, ssh agent based and null authentication. Libvirt's Auth callback is used for interaction with the user. (Keyboar interactive authentication, adding of host keys, private key passphrases). This enables seamless integration into the application using libvirt, as no helpers as ssh-askpass are needed. Reading and writing of OpenSSH style knownHosts files is supported. Communication is done using SSH exec channel, where the user may specify arbitrary command to be executed on the remote side and reads and writes to/from stdin/out are sent through the ssh channel. Usage of stderr is not supported (by this code). As a bonus, this should (untested) add SSH support to libvirt clients running on Windows. TODO: - add code for private key authentication - add code for SSH agent based authentication - add default paths for private key and known hosts file on unix systems - ask to add key to known host, if normal mode is selected. - change some error codes and messages to something more apropriate - tests? * configure.ac: Add checking for correct version and version dependent define * po/POTFILES.in: Add to translation. * src/Makefile.am: specify paths to new files * src/rpc/virnetlibsshcontext.c: Helpers for keeping ssh session context * src/rpc/virnetlibsshcontext.h: headers for above --- configure.ac | 40 ++- po/POTFILES.in|1 + src/Makefile.am |9 + src/rpc/virnetlibsshcontext.c | 900 + src/rpc/virnetlibsshcontext.h | 76 5 files changed, 1022 insertions(+), 4 deletions(-) create mode 100644 src/rpc/virnetlibsshcontext.c create mode 100644 src/rpc/virnetlibsshcontext.h diff --git a/configure.ac b/configure.ac index 3b7535e..955573f 100644 --- a/configure.ac +++ b/configure.ac @@ -73,6 +73,7 @@ OPENWSMAN_REQUIRED=2.2.3 LIBPCAP_REQUIRED=1.0.0 LIBNL_REQUIRED=1.1 LIBSSH2_REQUIRED=1.0 +LIBSSH2_TRANSPORT_REQUIRED=1.3 LIBBLKID_REQUIRED=2.17 dnl Checks for C compiler. @@ -305,6 +306,8 @@ AC_ARG_WITH([remote], AC_HELP_STRING([--with-remote], [add remote driver support @:@default=yes@:@]),[],[with_remote=yes]) AC_ARG_WITH([libvirtd], AC_HELP_STRING([--with-libvirtd], [add libvirtd support @:@default=yes@:@]),[],[with_libvirtd=yes]) +AC_ARG_WITH([libssh2_transport], + AC_HELP_STRING([--with-libssh2_transport], [libssh2 location @:@default=check@:@]),[],[with_libssh2_transport=check]) dnl dnl in case someone want to build static binaries @@ -1425,29 +1428,58 @@ AM_CONDITIONAL([WITH_UML], [test $with_uml = yes]) dnl -dnl check for libssh2 (PHYP) +dnl check for libssh2 (PHYP and libssh2 transport) dnl LIBSSH2_CFLAGS= LIBSSH2_LIBS= -if test $with_phyp = yes || test $with_phyp = check; then +if test $with_phyp = yes || test $with_phyp = check || + test $with_libssh2_transport = yes || test $with_libssh2_transport = check; then PKG_CHECK_MODULES([LIBSSH2], [libssh2 = $LIBSSH2_REQUIRED], [ -with_phyp=yes +if test $with_phyp = check; then +with_phyp=yes +fi +if $PKG_CONFIG libssh2 = $LIBSSH2_TRANSPORT_REQUIRED; then +if test $with_libssh2_transport = check; then +with_libssh2_transport=yes +fi +else +if test $with_libssh2_transport = check; then +with_libssh2_transport=no +AC_MSG_NOTICE([libssh2 = $LIBSSH2_TRANSPORT_REQUIRED is required for libssh2 transport]) +fi +if test $with_libssh2_transport = yes; then +AC_MSG_ERROR([libssh2 = $LIBSSH2_TRANSPORT_REQUIRED is required for libssh2 transport]) +fi +fi ], [ if test $with_phyp = check; then with_phyp=no AC_MSG_NOTICE([libssh2 is required for Phyp driver, disabling it]) -else +fi +if test $with_phyp = yes; then AC_MSG_ERROR([libssh2 = $LIBSSH2_REQUIRED is required for Phyp driver]) fi +if test $with_libssh2_transport = check; then +with_libssh2_transport=no +AC_MSG_NOTICE([libssh2 = $LIBSSH2_TRANSPORT_REQUIRED is required for libssh2 transport]) +fi +if test $with_libssh2_transport = yes; then +AC_MSG_ERROR([libssh2 = $LIBSSH2_TRANSPORT_REQUIRED is required for libssh2 transport]) +fi ]) fi if test $with_phyp = yes; then AC_DEFINE_UNQUOTED([WITH_PHYP], 1, [whether IBM HMC / IVM driver is enabled]) fi +if test $with_libssh2_transport = yes; then +AC_DEFINE_UNQUOTED([HAVE_LIBSSH2], 1, [wether libssh2 transport is enabled]) +fi + AM_CONDITIONAL([WITH_PHYP],[test $with_phyp = yes])
[libvirt] [PATCH TECHPREVIEW RFC 2/4] libssh2_transport: add ssh context support to virNetSocket
This patch enables virNetSocket to be used as an ssh client when
properly configured.
Fucntion virNetSocketNewConnectLibSSH() is added, that takes all needed
parameters and creates a libssh2 session context and performs steps
needed to open the connection.
* src/libvirt_private.syms: Export the new symbol.
* src/rpc/virnetsocket.c: Add virNetSocketNewConnectLibSSH
* src/rpc/virnetsocket.h: Add header.
---
src/libvirt_private.syms |1 +
src/rpc/virnetsocket.c | 137 +-
src/rpc/virnetsocket.h | 12
3 files changed, 149 insertions(+), 1 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b15a8db..af286d2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1301,6 +1301,7 @@ virNetSocketGetFD;
virNetSocketHasPassFD;
virNetSocketIsLocal;
virNetSocketListen;
+virNetSocketNewConnectLibSSH;
virNetSocketNewConnectTCP;
virNetSocketNewListenUNIX;
virNetSocketRecvFD;
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 30b8fe6..976a4ac 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -45,6 +45,10 @@
#include passfd.h
+#if HAVE_LIBSSH2
+# include virnetlibsshcontext.h
+#endif
+
#define VIR_FROM_THIS VIR_FROM_RPC
#define virNetError(code, ...)\
@@ -84,6 +88,9 @@ struct _virNetSocket {
size_t saslEncodedLength;
size_t saslEncodedOffset;
#endif
+#if HAVE_LIBSSH2
+virNetLibSSHSessionPtr sshSession;
+#endif
};
@@ -676,6 +683,98 @@ int virNetSocketNewConnectSSH(const char *nodename,
return virNetSocketNewConnectCommand(cmd, retsock);
}
+#if HAVE_LIBSSH2
+int virNetSocketNewConnectLibSSH(const char *host,
+ const char *port,
+ const char *username,
+ const char *password,
+ const char *command,
+ const char *knownHostsFile,
+ const char *hostkeyVerify,
+ const char *privkey,
+ virConnectAuthPtr auth,
+ virNetSocketPtr *retsock)
+{
+virNetSocketPtr sock = NULL;
+virNetLibSSHSessionPtr sess = NULL;
+int ret = -1;
+int portN;
+virNetLibSSHHostkeyVerify verify = VIR_NET_LIBSSH_HOSTKEY_VERIFY_NORMAL;
+
+
+if ((ret = virNetSocketNewConnectTCP(host, port, sock)) 0)
+goto error;
+
+sess = virNetLibSSHSessionNew();
+if (!sess){
+ret = -1;
+goto error;
+}
+
+/* configure libssh2 session */
+if ((ret = virNetLibSSHSessionSetCredentials(sess,
+ username,
+ password)) != 0)
+goto error;
+
+virNetLibSSHSessionSetAuthCallback(sess, auth); /* allways succeeds */
+
+
+if ((ret = virNetLibSSHSessionSetChannelCommand(sess, command)) != 0)
+goto error;
+
+if ((ret = virNetLibSSHSessionSetPrivateKey(sess, privkey)) != 0)
+goto error;
+
+/* port was verified while opening the socket */
+sscanf(port, %d, portN);
+
+if (hostkeyVerify) {
+if (STRCASEEQ(auto, hostkeyVerify))
+verify = VIR_NET_LIBSSH_HOSTKEY_VERIFY_AUTO_ADD;
+else if (STRCASEEQ(ignore, hostkeyVerify))
+verify = VIR_NET_LIBSSH_HOSTKEY_VERIFY_IGNORE;
+}
+
+if ((ret = virNetLibSSHSessionSetHostKeyVerification(sess,
+ host,
+ portN,
+ knownHostsFile,
+ false,
+ verify) != 0))
+goto error;
+
+/* connect to the host using ssh */
+if ((ret = virNetLibSSHSessionConnect(sess, virNetSocketGetFD(sock))) != 0)
+goto error;
+
+sock-sshSession = sess;
+*retsock = sock;
+
+return 0;
+
+error:
+virNetSocketFree(sock);
+virNetLibSSHSessionFree(sess);
+return ret;
+}
+#else
+int virNetSocketNewConnectLibSSH(const char *host ATTRIBUTE_UNUSED,
+ const char *port ATTRIBUTE_UNUSED,
+ const char *username ATTRIBUTE_UNUSED,
+ const char *password ATTRIBUTE_UNUSED,
+ const char *command ATTRIBUTE_UNUSED,
+ const char *knownHostsFile ATTRIBUTE_UNUSED,
+ const char *hostkeyVerify ATTRIBUTE_UNUSED,
+ const char *privkey ATTRIBUTE_UNUSED,
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+ virNetSocketPtr *retsock ATTRIBUTE_UNUSED)
+{
+
Re: [libvirt] [PATCH libvirt-glib 1/6] Add GVirDomainDevice abstract class
ACK 1-5, please don't push 6 for now as it may cause bad conflicts with some pending libvirt-gconfig changes. If you want to push the libvirt-gobject bits now, let me know, I'll look at it. Christophe On Mon, Nov 14, 2011 at 01:50:02PM +0100, Marc-André Lureau wrote: --- libvirt-gobject/Makefile.am|3 + .../libvirt-gobject-domain-device-private.h| 31 + libvirt-gobject/libvirt-gobject-domain-device.c| 139 libvirt-gobject/libvirt-gobject-domain-device.h| 64 + libvirt-gobject/libvirt-gobject.h |1 + libvirt-gobject/libvirt-gobject.sym|2 + 6 files changed, 240 insertions(+), 0 deletions(-) create mode 100644 libvirt-gobject/libvirt-gobject-domain-device-private.h create mode 100644 libvirt-gobject/libvirt-gobject-domain-device.c create mode 100644 libvirt-gobject/libvirt-gobject-domain-device.h diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am index 4f84b8b..56a047e 100644 --- a/libvirt-gobject/Makefile.am +++ b/libvirt-gobject/Makefile.am @@ -7,6 +7,7 @@ GOBJECT_HEADER_FILES = \ libvirt-gobject.h \ libvirt-gobject-main.h \ libvirt-gobject-domain-snapshot.h \ + libvirt-gobject-domain-device.h \ libvirt-gobject-domain.h \ libvirt-gobject-interface.h \ libvirt-gobject-network.h \ @@ -21,6 +22,7 @@ GOBJECT_HEADER_FILES = \ GOBJECT_SOURCE_FILES = \ libvirt-gobject-main.c \ libvirt-gobject-domain-snapshot.c \ + libvirt-gobject-domain-device.c \ libvirt-gobject-domain.c \ libvirt-gobject-interface.c \ libvirt-gobject-network.c \ @@ -42,6 +44,7 @@ libvirt_gobject_1_0_la_HEADERS = \ $(GOBJECT_HEADER_FILES) \ libvirt-gobject-input-stream.h nodist_libvirt_gobject_1_0_la_HEADERS = \ + libvirt-gobject-domain-device-private.h \ libvirt-gobject-enums.h libvirt_gobject_1_0_la_SOURCES = \ $(libvirt_gobject_1_0_la_HEADERS) \ diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h b/libvirt-gobject/libvirt-gobject-domain-device-private.h new file mode 100644 index 000..2a34309 --- /dev/null +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h @@ -0,0 +1,31 @@ +/* + * libvirt-gobject-domain-device-private.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 + * + * Author: Marc-André Lureau marcandre.lur...@redhat.com + */ +#ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_PRIVATE_H__ +#define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_PRIVATEH__ + +G_BEGIN_DECLS + +virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self); + +G_END_DECLS + +#endif /* __LIBVIRT_GOBJECT_DOMAIN_DEVICE_PRIVATEH__ */ diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c new file mode 100644 index 000..ae03489 --- /dev/null +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -0,0 +1,139 @@ +/* + * libvirt-gobject-domain-device.c: 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 + * + * Author: Marc-André Lureau
Re: [libvirt] [PATCH libvirt-glib 3/6] Add GVirDomainDisk
On Mon, Nov 14, 2011 at 01:50:04PM +0100, Marc-André Lureau wrote:
+/**
+ * gvir_domain_disk_get_stats:
+ * @self: the domain disk
+ * @err: an error
+ *
+ * This function returns network disk stats. Individual fields
+ * within the stats structure may be returned as -1, which indicates
+ * that the hypervisor does not support that particular statistic.
+ *
+ * Returns: (transfer full): the stats or %NULL in case of error
+ **/
+GVirDomainDiskStats *gvir_domain_disk_get_stats(GVirDomainDisk *self, GError
**err)
+{
+GVirDomainDiskStats *ret = NULL;
+virDomainBlockStatsStruct stats;
+GVirDomainDiskPrivate *priv;
+virDomainPtr handle;
+
+g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), NULL);
+
+priv = self-priv;
+handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self));
+
+if (virDomainBlockStats(handle, priv-path, stats, sizeof (stats)) 0)
{
Maybe we want to check if handle == NULL before calling into libvirt?
I checked that libvirt will return an error in this case.
(same question in patch 2/3)
(not an important issue, my ACK series still stand whatever you decide to
do about this)
Christophe
pgpNQoJJrLOju.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: forbid rebuilding existing disk storage pools
On Mon, Nov 14, 2011 at 12:58:08PM +0800, Osier Yang wrote:
于 2011年11月13日 00:19, Guido Günther 写道:
which would blow away all volumes. Honor VIR_STORAGE_POOL_BUILD_OVERWRITE
to force a rebuild.
This was caught by libvirt-tck's storage/110-disk-pool.t.
Cheers,
-- Guido
---
src/storage/storage_backend_disk.c | 72
++--
1 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_backend_disk.c
b/src/storage/storage_backend_disk.c
index 82d6e8a..995ad2f 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -335,6 +335,40 @@ virStorageBackendDiskRefreshPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
/**
+ * Check for a valid disk label (partition table) on device
+ *
+ * return: 0 - valid disk label found
+ *0 - no or unrecognized disk label
+ *0 - error finding the disk label
+ */
+static int
+virStorageBackendDiskFindLabel(const char* device)
+{
+const char *const args[] = {
+device, print, --script, NULL,
+};
+virCommandPtr cmd = virCommandNew(PARTED);
+char *output = NULL;
+int ret = -1;
+
+virCommandAddArgSet(cmd, args);
+virCommandAddEnvString(cmd, LC_ALL=C);
+virCommandSetOutputBuffer(cmd,output);
+
+/* if parted succeeds we have a valid partition table */
+ret = virCommandRun(cmd, NULL);
+if (ret 0) {
+if (strstr (output, unrecognised disk label))
+ret = 1;
+}
+
+virCommandFree(cmd);
+VIR_FREE(output);
+return ret;
+}
+
+
+/**
* Write a new partition table header
*/
static int
@@ -342,6 +376,8 @@ virStorageBackendDiskBuildPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool,
unsigned int flags)
{
+bool ok_to_mklabel = false;
+int ret = -1;
/* eg parted /dev/sda mklabel msdos */
const char *prog[] = {
PARTED,
@@ -353,12 +389,40 @@ virStorageBackendDiskBuildPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
NULL,
};
-virCheckFlags(0, -1);
+virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
+ VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
-if (virRun(prog, NULL) 0)
-return -1;
+if (flags == (VIR_STORAGE_POOL_BUILD_OVERWRITE |
+ VIR_STORAGE_POOL_BUILD_NO_OVERWRITE)) {
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ _(Overwrite and no overwrite flags
+ are mutually exclusive));
+goto error;
+}
-return 0;
+if (flags VIR_STORAGE_POOL_BUILD_OVERWRITE)
+ok_to_mklabel = true;
+else {
+int check;
+
+check = virStorageBackendDiskFindLabel (
+pool-def-source.devices[0].path);
+if (check 0) {
+ok_to_mklabel = true;
+} else if (check 0) {
+virStorageReportError(VIR_ERR_OPERATION_FAILED,
+ _(Error checking for disk label));
+} else {
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ _(Disk label already present));
+}
+}
+
+if (ok_to_mklabel)
+ret = virRun(prog, NULL);
+
+error:
+return ret;
}
/**
Looks fine, and having the flags is better, ACK
Pushed. Thanks.
-- Guido
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] blkiotune: add interface for blkiotune.device_weight
On 11/08/2011 05:43 AM, Stefan Berger wrote:
On 11/08/2011 06:00 AM, Hu Tao wrote:
This adds per-device weights toblkiotune. Note that the
cgroups implementation only supports weights per block device,
and not per-file within the device; hence this option must be
global to the domain definition rather than tied to individual
device/disk entries:
domain ...
blkiotune
device
path/path/to/block/path
weight1000/weight
/device
/blkiotune
...
This patch also adds a parameter --device-weight to virsh command
blkiotune for setting/getting blkiotune.weight_device for any
hypervisor that supports it. Alldevice entries under
blkiotune are concatenated into a single string attribute under
virDomain{Get,Set}BlkioParameters, named device_weight.
+/**
+ * VIR_DOMAIN_BLKIO_DEVICE_WEIGHT:
+ *
+ * Macro for the blkio tunable weight_device: it represents the
+ * per-device weight, as a string. The string is parsed as a
+ * series of /path/to/device,weight elements, separated by ';'.
It looks like the rest of the code swapped over to separated by ',',
so I made that tweak.
+ * This function returns a string representing device weights that is
+ * suitable for writing to /cgroup/blkio/blkio.weight_device, given
+ * a list of per-device weights.
+ */
+#if defined(major) defined(minor)
+int virBlkioDeviceWeightToStr(virBlkioDeviceWeightPtr devices,
+ int ndevices,
+ char **result)
+{
+int i;
+struct stat s;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+for (i = 0; i ndevices; i++) {
+if (stat(devices[i].path,s) == -1)
+return -1;
+if ((s.st_mode S_IFMT) != S_IFBLK)
+return -1;
+virBufferAsprintf(buf, %d:%d %d\n,
+ major(s.st_rdev),
+ minor(s.st_rdev),
+ devices[i].weight);
+}
+
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+return -1;
+}
+
+*result = virBufferContentAndReset(buf);
+return 0;
You fail for more than one reason, but don't output a failure log
message, which means the caller has to do it and can't guess why things
failed (OOM? not a block device?). I don't see any callers in this
patch, so I checked 5/5; there, both callers just reported an internal
error stating the string could not be parsed. But internal error for a
user-visible message is rather harsh, so I think this is worth improving.
+}
+#else
+int virBlkioDeviceWeightToStr(virBlkioDeviceWeightPtr devices
ATTRIBUTE_UNUSED,
+ int ndevices ATTRIBUTE_UNUSED,
+ char **result ATTRIBUTE_UNUSED)
+{
+return -1;
And if we improve the error reporting above, then we also have to report
an error here.
+static int virDomainBlkioDeviceWeightParseXML(xmlNodePtr root,
+ virBlkioDeviceWeightPtr dw)
+{
+char *c;
+xmlNodePtr node;
+
+if (!dw)
+return -1;
Dead check - this is a static function, and we know the caller gives us
valid inputs.
+node = root-children;
+while (node) {
+if (node-type == XML_ELEMENT_NODE) {
+if (xmlStrEqual(node-name, BAD_CAST path)) {
+dw-path = (char *)xmlNodeGetContent(node);
Memory leak; if the user (mistakenly) has more than one path
subelement, then you are blindly overwriting dw-path on the second
instance of path.
+} else if (xmlStrEqual(node-name, BAD_CAST weight)) {
+c = (char *)xmlNodeGetContent(node);
+if (virStrToLong_ui(c, NULL, 10,dw-weight) 0) {
+VIR_FREE(c);
+VIR_FREE(dw-path);
+return -1;
Missing error reporting - either we must report the error here or in the
caller (I elected for here, to match precedence with other functions
used by the caller).
+}
+VIR_FREE(c);
+}
+}
+node = node-next;
+}
You never validated that path was a mandatory element.
@@ -6711,6 +6813,24 @@ static virDomainDefPtr
virDomainDefParseXML(virCapsPtr caps,
def-blkio.weight) 0)
def-blkio.weight = 0;
+n = virXPathNodeSet(./blkiotune/device, ctxt,nodes);
+if (n 0) {
Missing an OOM memory check.
+if (VIR_ALLOC_N(def-blkio.devices, n) 0) {
+virReportOOMError();
+goto error;
+}
+
+for (i = 0; i n; i++) {
+if (virDomainBlkioDeviceWeightParseXML(nodes[i],
+def-blkio.devices[i]) 0) {
+virBlkioDeviceWeightArrayClear(def-blkio.devices, i);
Redundant, if you had been updating def-blkio.ndevices as you go, for
consistency with some of the other clients of virXPathNodeSet in this
method.
+++ b/tools/virsh.c
@@ -4648,6
[libvirt] [PATCH TECHPREVIEW RFC 3/4] libssh2_transport: Add libssh2 session support to net client code
This patch adds a glue layer to enable using libssh2 code with the
network client code.
As in the original client implementation, shell code is sent to the
server to detect correct options for netcat.
*src/rpc/virnetclient.c:
*src/rpc/virnetclient.h: Add function to handle connection to a libvirt
daemon using the libssh2 transport.
---
src/rpc/virnetclient.c | 66
src/rpc/virnetclient.h | 11
2 files changed, 77 insertions(+), 0 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 4b7d4a9..078bec3 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -216,6 +216,72 @@ virNetClientPtr virNetClientNewSSH(const char *nodename,
return virNetClientNew(sock, NULL);
}
+virNetClientPtr virNetClientNewLibSSH(const char *host,
+ const char *port,
+ const char *username,
+ const char *password,
+ const char *netcat,
+ const char *socketPath,
+ const char *knownHostsFile,
+ const char *hostkeyVerify,
+ const char *privkey,
+ virConnectAuthPtr auth)
+{
+virNetSocketPtr sock;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *nc = NULL;
+
+if (!host)
+host = localhost;
+
+if (!port)
+port = 22;
+
+if (!username)
+username = root;
+
+if (netcat) {
+virBufferEscapeShell(buf, netcat);
+nc = virBufferContentAndReset(buf);
+} else {
+nc = strdup(nc);
+}
+
+if (!nc) {
+virReportOOMError();
+return NULL;
+}
+
+virBufferAsprintf(buf,
+ sh -c
+ 'if '%s' -q 21 | grep \requires an argument\ /dev/null 21;
then
+ ARG=-q0;
+ else
+ ARG=;
+ fi;
+ '%s' $ARG -U %s',
+ nc, nc, socketPath);
+
+VIR_FREE(nc);
+
+if (virBufferError(buf)) {
+virReportOOMError();
+return NULL;
+}
+
+nc = virBufferContentAndReset(buf);
+
+if (virNetSocketNewConnectLibSSH(host, port, username, password, nc,
+ knownHostsFile, hostkeyVerify,
+ privkey, auth, sock) 0) {
+VIR_FREE(nc);
+return NULL;
+}
+
+VIR_FREE(nc);
+return virNetClientNew(sock, NULL);
+}
+
virNetClientPtr virNetClientNewExternal(const char **cmdargv)
{
virNetSocketPtr sock;
diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h
index fb679e8..4b494a1 100644
--- a/src/rpc/virnetclient.h
+++ b/src/rpc/virnetclient.h
@@ -49,6 +49,17 @@ virNetClientPtr virNetClientNewSSH(const char *nodename,
const char *keyfile,
const char *path);
+virNetClientPtr virNetClientNewLibSSH(const char *host,
+ const char *port,
+ const char *username,
+ const char *password,
+ const char *netcat,
+ const char *socketPath,
+ const char *knownHostsFile,
+ const char *hostkeyVerify,
+ const char *privkey,
+ virConnectAuthPtr auth);
+
virNetClientPtr virNetClientNewExternal(const char **cmdargv);
void virNetClientRef(virNetClientPtr client);
--
1.7.3.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/33] Rename virVirtualPortProfileParams APIs
On Wed, Nov 09, 2011 at 02:12:02AM -0500, Laine Stump wrote:
On 11/03/2011 01:30 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrangeberra...@redhat.com
Rename the virVirtualPortProfileParams struct to be
virNetDevVPortProfile, and rename the APIs to match
this prefix.
* src/util/network.c, src/util/network.h: Rename port profile
APIs
* src/conf/domain_conf.c, src/conf/domain_conf.h,
src/conf/network_conf.c, src/conf/network_conf.h,
src/network/bridge_driver.c, src/qemu/qemu_hotplug.c,
src/util/macvtap.c, src/util/macvtap.h: Update for
renamed APIs/structs
---
src/conf/domain_conf.c | 16 +++---
src/conf/domain_conf.h |8 +++---
src/conf/network_conf.c | 12 +-
src/conf/network_conf.h |4 +-
src/libvirt_private.syms|6 ++--
src/network/bridge_driver.c |6 ++--
src/qemu/qemu_hotplug.c |4 +-
src/util/macvtap.c | 36
src/util/macvtap.h |8 +++---
src/util/network.c | 48
+-
src/util/network.h | 33 +++--
11 files changed, 91 insertions(+), 90 deletions(-)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index cb13d2b..71243b8 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -89,7 +89,7 @@ VIR_ENUM_IMPL(virMacvtapMode, VIR_MACVTAP_MODE_LAST,
# define LLDPAD_PID_FILE /var/run/lldpad.pid
-enum virVirtualPortOp {
+enum virNetDevVPortOp {
ASSOCIATE = 0x1,
DISASSOCIATE = 0x2,
PREASSOCIATE = 0x3,
Do you think having such generic names for these enums might lead to
a namespace conflict somewhere down the road? Maybe the enum value
names could be changed as a part of this patch...
The enum itself later gets renamed again to
virNetDevVPortProfileLinkOp
I'm moving the rename to this patch, and also renaming the constants
to match by having a prefix VIR_NETDEV_VPORT_PROFILE_LINK_OP. Yes it
is a little verbose, but these are only used in a handful of places.
-#endif /* WITH_MACVTAP || WITH_VIRTUALPORT */
+#endif /* WITH_MACVTAP || WITH_NETDEV_VPORT_PROFILE */
WITH_VIRTUALPORT has been changed to WITH_NETDEV_VPORT_PROFILE in
this comment, but not in the #ifdef, and the line was completely
removed in PATCH 19/33. Since the latter name doesn't appear
anywhere in the final result of the series, I think this must be a
vestige of something you later decided against, or maybe a
search-replace run amok.
Yes, bogus search replace I have reverted.
ACK aside from 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] [PATCH 12/33] Fix error reporting in port profile parsing/formatting APIs
On Wed, Nov 09, 2011 at 02:58:32AM -0500, Laine Stump wrote:
After applying this patch, make fails with:
CC libvirt_util_la-network.lo
cc1: warnings being treated as errors
util/network.c: In function 'virNetDevVPortProfileParse':
util/network.c:712:23: error: assignment makes pointer from integer
without a cast
util/network.c:712:69: error: ordered comparison of pointer with
integer zero [-Wextra]
On 11/03/2011 01:30 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrangeberra...@redhat.com
The virtual port profile parsing/formatting APIs do not
correctly handle unknown profile type strings/numbers.
They behave as a no-op, instead of raising an error
Actually I've noticed a *lot* of the *Format functions ignore the
possibility of bad values in the vir*Def objects (e.g. invalid enum
values), and have debated with myself about whether to ignore or
report invalid data.
If we work from the assumption that the vir*Def objects can only be
populated as a result of parsing an XML document, then we can assume
the enum values in the vir*Def are all valid within range.
If we allow for the possibility that code can populate the vir*Def
objects programmatically, then if it exclusively uses the enum
constants we will again be safe.
Only if we somehow populate vir*Def objects directly using int
values, instead of constants or the formal string-int conversion
APIs, do we need to consider invalid values.
This particular code *was* assuming the worst and explicitly trying
to handle bogus values, but doing so in a really lame manner.
IMHO it is not neccessary to handle invalid enum values in the
formatting code, but if you do decide to handle them, then you
*must* report the errors correctly and not just pretend they
don't exist after you detected them. The latter is what this
patch is fixing.
@@ -700,13 +699,19 @@ virNetDevVPortProfileParse(xmlNodePtr node,
if (VIR_ALLOC(virtPort) 0) {
virReportOOMError();
-return -1;
+return NULL;
}
virtPortType = virXMLPropString(node, type);
if (!virtPortType) {
virSocketError(VIR_ERR_XML_ERROR, %s,
- _(missing virtualportprofile type));
+ _(missing virtualportprofile type));
+goto error;
+}
The following should be virtPort-virPortType = :
Opps, yes. Fixed in a later patch, will pull the fix back here.
Anyway, ACK with the compile problem fixed.
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: Fix build without MACVTAP
On 11/12/2011 06:39 AM, Michael Wood wrote: Hi Commit c31d23a78715f1144c73862c46ab0436de8b5e85 removed the conn parameter from qemuPhysIfaceConnect(), but it's still used if WITH_MACVTAP is false. Also, it's still mentioned in the comment above the function: From f4fc43b4111a4c099395c55902e497b8965e2b53 Mon Sep 17 00:00:00 2001 From: Michael Wood esiot...@gmail.com Date: Sat, 12 Nov 2011 13:37:53 +0200 Subject: [PATCH] Fix build without MACVTAP. --- src/qemu/qemu_command.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b044050..2fbf691 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -126,10 +126,10 @@ uname_normalize (struct utsname *ut) /** * qemuPhysIfaceConnect: * @def: the definition of the VM (needed by 802.1Qbh and audit) - * @conn: pointer to virConnect object * @driver: pointer to the qemud_driver * @net: pointer to he VM's interface description with direct device type * @qemuCaps: flags for qemu + * @vmop: VM operation type * * Returns a filedescriptor on success or -1 in case of error. */ @@ -165,7 +165,6 @@ qemuPhysIfaceConnect(virDomainDefPtr def, #else (void)def; -(void)conn; (void)net; (void)qemuCaps; (void)driver; ACK. I'll push it and add you as an author. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 0/4] Libvirt for PowerPC
On 11/14/2011 09:43 AM, Prerna Saxena wrote: Hi, Recent development in KVM for 64-bit Power ISA Book3S machines, allows users to run multiple KVM guest instances on POWER7 and PPC970 processor based systems. Also qemu-system-ppc64 has been enhanced to support a new machine type pseries suitable for Power Book3S machines. This addition effectively brings the KVM+qemu combination to run multiple guest instances on a Power Book3S machine. Libvirt continues to be the key interface to configure and manage the KVM guest instances on x86. This patch set is an effort to enable libvirt to support KVM guest configuration and management on Power Book3S machines. Based on community discussion around the earlier version, this patch series augments the present 'kvm' driver to support PowerPC-KVM based guests. Since some of the supported devices vary between architectures, the code is now reworked to dynamically choose the correct handler function based on guest domain architecture at runtime (def-os.arch). This patch series consists of 4 patches : Patch 1/4 : Use sysfs to gather host topology. Presently libvirt depends on /proc/cpuinfo gather CPU, cores, threads, etc This is highly architecture dependent. A alternative is to use sysfs, which provides a platform-neutral interface to parse CPU topology. Patch 2/4 : Add PowerPC CPU Driver Patch 3/4 : Add support for qemu-system-ppc64 Patch 4/4 : Refactor qemu commmand-line generation to separate out arch- specific options from generic command line. Changelog from v1: * Patches 1,2,3 unchanged ; The hacks in Patch 4 of v1 have been replaced by a new patch to neatly select arch-specific features. Awaiting comments and feedback, A few quick comments: Run 'make syntax-check'. Some cleanup needs to be done. Also use ATTRIBUTE_UNUSED on variables that are not use, like here when compiling on x86. cpu/cpu_powerpc.c: In function 'PowerPCDecode': cpu/cpu_powerpc.c:50:28: warning: unused parameter 'cpu' [-Wunused-parameter] cpu/cpu_powerpc.c:51:36: warning: unused parameter 'data' [-Wunused-parameter] cpu/cpu_powerpc.c:52:28: warning: unused parameter 'models' [-Wunused-parameter] cpu/cpu_powerpc.c:53:28: warning: unused parameter 'nmodels' [-Wunused-parameter] cpu/cpu_powerpc.c:54:27: warning: unused parameter 'preferred' [-Wunused-parameter] cpu/cpu_powerpc.c: At top level: cpu/cpu_powerpc.c:75:5: warning: initialization from incompatible pointer type cpu/cpu_powerpc.c: In function 'PowerPCDataFree': cpu/cpu_powerpc.c:66:1: warning: control reaches end of non-void function [-Wreturn-type] nodeinfo.c: In function 'linuxNodeInfoCPUPopulate': nodeinfo.c:287:5: warning: suggest parentheses around assignment used as truth value [-Wparentheses] nodeinfo.c:202:36: warning: unused parameter 'need_hyperthreads' [-Wunused-parameter] qemu/qemu_command.c: In function 'qemuBuildX86CommandLine': qemu/qemu_command.c:5250:17: warning: too many arguments for format [-Wformat-extra-args] qemu/qemu_command.c:5201:41: warning: unused parameter 'conn' [-Wunused-parameter] qemu/qemu_command.c: In function 'qemuBuildCommandLine': qemu/qemu_command.c:3264:35: warning: 'qemuCmdFunc' may be used uninitialized in this function [-Wuninitialized] Some of the code (part 4) when I looked seemed to have indentation problems. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib PATCH] Add vapi dependency to Makefile.am and fix parallel build
From: Nirbheek Chauhan nirbh...@gentoo.org libvirt-glib-1.0.vapi is needed to generate libvirt-gobject-1.0.vapi Without an explicit dependency, make -jN fails Signed-off-by: Nirbheek Chauhan nirbh...@gentoo.org --- vapi/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vapi/Makefile.am b/vapi/Makefile.am index 0af8f6f..2ecc3e3 100644 --- a/vapi/Makefile.am +++ b/vapi/Makefile.am @@ -17,7 +17,7 @@ libvirt-glib-1.0.vapi: $(top_builddir)/libvirt-glib/LibvirtGLib-1.0.gir --library libvirt-glib-1.0 \ $ -libvirt-gobject-1.0.vapi: $(top_builddir)/libvirt-gobject/LibvirtGObject-1.0.gir +libvirt-gobject-1.0.vapi: $(top_builddir)/libvirt-gobject/LibvirtGObject-1.0.gir libvirt-glib-1.0.vapi $(AM_V_GEN)$(VAPIGEN) \ --vapidir=$(builddir) \ --pkg gobject-2.0 \ -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Bridged Networking Wiki Up-to-Date?
? 2011?11?13? 10:38, Bob Cochran ??: Greetings, I just installed Fedora 16 on a new server and want to create a Fedora 16 virtual machine. I have not created or used VMs on Fedora in quite some time. Now I have several compelling reasons to use Fedora virtualization so I am kind of coming back to it. Is the libvirt bridged networking wiki up-to-date? http://wiki.libvirt.org/page/Networking I thinks you could just follow this section: http://wiki.libvirt.org/page/Networking#Fedora.2FRHEL_Bridging My hope is that the default networking support provided on Fedora 16 will let any virtual guest connect to any other host on my internal LAN (within reason, of course) so that for example one virtual guest can be dedicated as a DNS server and all the other hosts on my network can query it. That is, I hope I won't need to do extensive additional networking support when I create the guest OS, but I am willing to if I have to. Thanks Bob Cochran -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- best regards eli -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume
On 11/13/2011 09:08 PM, MATSUDA, Daiki wrote: NACK. There is nothing inherently wrong with the source file not being a qcow2 file. The whole point of creating a runtime snapshot is that the original file (of _any_ format) becomes the backing file of a new qcow2 file, so that the original file now serves as the snapshot. Forbidding a live snapshot of a raw source file interferes with this intent. I have some tested since you replied. Certainly other image file, e.g. raw type, has no problem after snapshot is taken in qcow2 format. But in the case that direct disk block, e.g. /dev/sdc, is given for the guest OS, the same error occurs. It may not be accepted by qemu-kvm. I'm not following you. As I already said, there's nothing wrong with a direct disk block as the backing file of a qcow2 image. I do not understand which qemu-kvm or libvirt should be fixed. But at least libvirt should be stop to take snapshot for block device with following patch. I still say NACK. Your patch is merely attempting to forbid a useful case, rather than fix a demonstrated problem. If I recall correctly, you started this thread when you used 'virsh snapshot-create --disk-only' without arguments, and thus fell victim to virsh picking the snapshot name for you, but trying to pick that name under /dev instead of a more typical location for a non-device file. But that's a usage error in you not taking time to provide virsh with the alternate name to use for the qcow2 file, and not a flaw that needs correcting in libvirt itself. Or even if there is a flaw in libvirt, the fix is not to forbid snapshots of a raw block device, but to be smarter about ensuring that any generated qcow2 file name is likely to be correct (a qcow2 file created under /dev probably only makes sense if the qcow2 data is being written atop a pre-existing block device, rather than trying to use open(O_CREAT) to create a regular file in /dev). -- 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 5/5] blkiotune: add qemu support for blkiotune.device_weight
On 11/08/2011 06:05 AM, Stefan Berger wrote:
On 11/08/2011 06:00 AM, Hu Tao wrote:
Implement setting/getting per-device blkio weights in qemu,
using the cgroups blkio.weight_device tunable.
---
src/libvirt_private.syms |1 +
src/qemu/qemu_cgroup.c | 22 +
src/qemu/qemu_driver.c | 216
-
src/util/cgroup.c| 20
src/util/cgroup.h|3 +
5 files changed, 257 insertions(+), 5 deletions(-)
In addition to Stefan's comments, which I fixed as recommended, I had
some additional comments. In particular, qemu's set blkio parameters
had a pre-existing bug, where using 'virsh blkiotune --live
--persistent' failed to update persistent state, which I noticed in
testing this (a simple matter of removing a bogus 'else').
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -312,6 +312,28 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
}
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_BLKIO)) {
+char *tmp;
+if (virBlkioDeviceWeightToStr(vm-def-blkio.devices,
+ vm-def-blkio.ndevices,
+tmp) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Unable to set io device weight for
domain %s),
+vm-def-name);
The ToStr function doesn't report an OOM error. So this is ok. I wonder
whether that function should report an OOM error, though? Eric?
I fixed that in my review of 4/5 to have ToStr always report error, so
no need to report error here.
+goto cleanup;
+}
+if (tmp) {
I think you can remove the if branch.
Not entirely true. virBlkioDeviceWeightToStr sets tmp to NULL and
returns 0 if vm-def-blkio.ndevices is 0. But in that case, why even
bother going into this entire block of code?
@@ -5978,6 +6058,53 @@ static int
qemuDomainSetBlkioParameters(virDomainPtr dom,
_(unable to set blkio
weight tunable));
ret = -1;
}
+} else if (STREQ(param-field,
VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
+int ndevices;
+virBlkioDeviceWeightPtr devices = NULL;
+char *tmp;
+if (param-type != VIR_TYPED_PARAM_STRING) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(invalid type for device_weight
tunable, expected a 'char *'));
+ret = -1;
+continue;
+}
+
+if (parseBlkioWeightDeviceStr(params[i].value.s,
+devices,
+ndevices) 0) {
+ret = -1;
+continue;
+}
+if (virBlkioDeviceWeightToStr(devices,
+ ndevices,
+tmp) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Unable to set blkio
weight_device tunable));
+virBlkioDeviceWeightArrayClear(devices, ndevices);
+VIR_FREE(devices);
+ret = -1;
+continue;
+}
+if (tmp) {
Useless conditional; here we know tmp is non-NULL because
virBlkioDeviceWeightToStr succeeded.
ACK with those nits fixed.
In my testing, I ran into a weird issue. From the shell:
printf '8:0 500\n8:16 500\n'
/cgroup/blkio/libvirt/qemu/dom/blkio.weight_device
succeeded at altering the cgroup contents. But:
/usr/bin/printf '8:0 500\n8:16 500\n'
/cgroup/blkio/libvirt/qemu/dom/blkio.weight_device
fails, as did the C code, with EINVAL. It took me longer than I care to
admit to realize the issue: bash's printf uses line-buffering mode on
ALL files, and results in two separate write() calls of one line each,
while coreutils' printf and our C code was trying to do two lines at
once and getting rejected by the kernel.
Therefore, we need to split up the code to use cgroups.c for only one
device at a time. Also, when thinking about it more, domain_conf.c is
the wrong place to worry about major() and minor(); util/cgroup.c
already worries about it. So I'm going to propose a v8 of the last two
patches, moving a hunk out of 4/5 into 5/5 so that we only use major()
and minor() in cgroup-specific code, rather than globally in domain_conf.c.
Here's where I got prior to hitting the EINVAL issue with multiple
blocks; I'll be posting further edits in the form of v8 soon.
diff --git i/src/qemu/qemu_cgroup.c w/src/qemu/qemu_cgroup.c
index d397578..99b8105 100644
--- i/src/qemu/qemu_cgroup.c
+++ w/src/qemu/qemu_cgroup.c
@@ -312,7 +312,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
}
-if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+if (vm-def-blkio.ndevices
+
Re: [libvirt] Non-zero constant warning on RHEL 6.1 with 0.9.7
On 11/11/2011 04:54 PM, Justin Clift wrote: On 12/11/2011, at 8:06 AM, Eric Blake wrote: On 11/08/2011 11:46 PM, Justin Clift wrote: Hi guys, Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3: CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] Obviously not fatal, but it figured someone might want to keep things warning free. :) This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :( H, we might as well report it, so it gets looked at. Next question is where? Upstream gcc, or on the RG BZ as a Fedora15(+?)/RHEL6 specific gcc bug? I am seeing it in this gcc version: Using built-in specs. Target: ppc64-redhat-linux Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-languages=c,c++,objc,obj-c++,java,fortran,ada --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-ppl --with-cloog --enable-secureplt --with-long-double-128 --with-cpu-32=power4 --with-tune-32=power6 --with-cpu-64=power4 --with-tune-64=power6 --build=ppc64-redhat-linux Thread model: posix gcc version 4.4.5 20110214 (Red Hat 4.4.5-6) (GCC) Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Non-zero constant warning on RHEL 6.1 with 0.9.7
On 11/11/2011 04:06 PM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling
with make -j 3:
CC libvirt_lxc-command.o
util/buf.c: In function 'virBufferEscape':
util/buf.c:469: warning: logical '' with non-zero constant will always
evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning
free. :)
This is a bug in gcc, although I'm not sure if anyone has raised a bug
report against the gcc folks yet:
https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need
to disable -Wlogical-op if glibc's headers are turning on the optimized
strchr (is that -D_FORTIFY_SOURCE that does it?) :(
This should work, no?
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -466,7 +466,8 @@ virBufferEscape(virBufferPtr buf, const char *toescape,
cur = str;
out = escaped;
while (*cur != 0) {
-if (strchr(toescape, *cur))
+char x[2] = { *cur, 0 };
+if (strstr(toescape, x))
*out++ = '\\';
*out++ = *cur;
cur++;
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 0/4] Libvirt for PowerPC
On 11/15/2011 02:36 AM, Stefan Berger wrote: On 11/14/2011 09:43 AM, Prerna Saxena wrote: Hi, Recent development in KVM for 64-bit Power ISA Book3S machines, allows users to run multiple KVM guest instances on POWER7 and PPC970 processor based systems. Also qemu-system-ppc64 has been enhanced to support a new machine type pseries suitable for Power Book3S machines. This addition effectively brings the KVM+qemu combination to run multiple guest instances on a Power Book3S machine. Libvirt continues to be the key interface to configure and manage the KVM guest instances on x86. This patch set is an effort to enable libvirt to support KVM guest configuration and management on Power Book3S machines. Based on community discussion around the earlier version, this patch series augments the present 'kvm' driver to support PowerPC-KVM based guests. Since some of the supported devices vary between architectures, the code is now reworked to dynamically choose the correct handler function based on guest domain architecture at runtime (def-os.arch). This patch series consists of 4 patches : Patch 1/4 : Use sysfs to gather host topology. Presently libvirt depends on /proc/cpuinfo gather CPU, cores, threads, etc This is highly architecture dependent. A alternative is to use sysfs, which provides a platform-neutral interface to parse CPU topology. Patch 2/4 : Add PowerPC CPU Driver Patch 3/4 : Add support for qemu-system-ppc64 Patch 4/4 : Refactor qemu commmand-line generation to separate out arch- specific options from generic command line. Changelog from v1: * Patches 1,2,3 unchanged ; The hacks in Patch 4 of v1 have been replaced by a new patch to neatly select arch-specific features. Awaiting comments and feedback, A few quick comments: Run 'make syntax-check'. Some cleanup needs to be done. Also use ATTRIBUTE_UNUSED on variables that are not use, like here when compiling on x86. cpu/cpu_powerpc.c: In function 'PowerPCDecode': cpu/cpu_powerpc.c:50:28: warning: unused parameter 'cpu' [-Wunused-parameter] cpu/cpu_powerpc.c:51:36: warning: unused parameter 'data' [-Wunused-parameter] cpu/cpu_powerpc.c:52:28: warning: unused parameter 'models' [-Wunused-parameter] cpu/cpu_powerpc.c:53:28: warning: unused parameter 'nmodels' [-Wunused-parameter] cpu/cpu_powerpc.c:54:27: warning: unused parameter 'preferred' [-Wunused-parameter] cpu/cpu_powerpc.c: At top level: cpu/cpu_powerpc.c:75:5: warning: initialization from incompatible pointer type cpu/cpu_powerpc.c: In function 'PowerPCDataFree': cpu/cpu_powerpc.c:66:1: warning: control reaches end of non-void function [-Wreturn-type] nodeinfo.c: In function 'linuxNodeInfoCPUPopulate': nodeinfo.c:287:5: warning: suggest parentheses around assignment used as truth value [-Wparentheses] nodeinfo.c:202:36: warning: unused parameter 'need_hyperthreads' [-Wunused-parameter] qemu/qemu_command.c: In function 'qemuBuildX86CommandLine': qemu/qemu_command.c:5250:17: warning: too many arguments for format [-Wformat-extra-args] qemu/qemu_command.c:5201:41: warning: unused parameter 'conn' [-Wunused-parameter] qemu/qemu_command.c: In function 'qemuBuildCommandLine': qemu/qemu_command.c:3264:35: warning: 'qemuCmdFunc' may be used uninitialized in this function [-Wuninitialized] Some of the code (part 4) when I looked seemed to have indentation problems. Stefan Hi Stefan, Thank you for your comments. I will fix these warnings, and use ATTRIBUTE_UNUSED for variables not in use. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Non-zero constant warning on RHEL 6.1 with 0.9.7
On 11/14/2011 05:54 PM, Stefan Berger wrote:
On 11/11/2011 04:06 PM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling
with make -j 3:
CC libvirt_lxc-command.o
util/buf.c: In function 'virBufferEscape':
util/buf.c:469: warning: logical '' with non-zero constant will
always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things
warning free. :)
This is a bug in gcc, although I'm not sure if anyone has raised a bug
report against the gcc folks yet:
https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need
to disable -Wlogical-op if glibc's headers are turning on the optimized
strchr (is that -D_FORTIFY_SOURCE that does it?) :(
This should work, no?
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -466,7 +466,8 @@ virBufferEscape(virBufferPtr buf, const char *toescape,
cur = str;
out = escaped;
while (*cur != 0) {
-if (strchr(toescape, *cur))
+char x[2] = { *cur, 0 };
+if (strstr(toescape, x))
The glibc headers are expanding the pre-patch code to:
if ((__extension__ (__builtin_constant_p (*cur)
!__builtin_constant_p (toescape) (*cur) == '\0' ? (char *)
__rawmemchr (toescape, *cur) : __builtin_strchr (toescape, *cur
but warning about:
util/buf.c:469: warning: logical '' with non-zero constant will
always evaluate as true [-Wlogical-op]
I see two constants in that expression:
!__builtin_constant_p (toescape) is always 1
(*cur) == '\0' is always 0 (since the containing while loop already
guaranteed *cur != 0)
There's definitely at least one gcc bug: that of an incorrect error
message. After all, the expression 'expra exprb' is constant in only
three cases: 'expra' is false, 'exprb' is false, or 'expra' is true and
'exprb' is true. But the gcc message isn't clear on whether it is
warning about 'expra' or 'exprb', not to mention that it claims the
overall expression is true even though we just proved the overall
expression is always false by virtue of exprb being false.
Whether there is a secondary bug, about gcc being noisy with
-Wlogical-op inside __extension__, is also worth arguing.
At any rate, I agree that your proposed patch skirts the issue by using
strstr() instead of strchr(), so it would indeed do the trick (while
under the hood, strstr() on a single-byte needle tends to defer to
strchr() anyways). It feels kind of gross, without at least a comment
explaining that we are working around a gcc bug in -Wlogical-op (and
even better, a URL of the gcc bug report); but I'm inclined to ACK it if
we can come up with an appropriate comment.
An even more aggressive patch would use strpbrk() to locate each
character that needs escaping and memcpy() to quickly copy chunks of
non-escaped text, rather than processing things one byte at a time, but
I'm not sure whether that would be a micro-optimization or whether this
function is called often enough to make it worth the extra complexity.
What does 'make V=1' show as the exact compiler line used when
triggering the error? I still haven't been able to reproduce the bogus
compiler warning on a much smaller test case, but suspect I may be
missing a particular flag that makes it apparent.
--
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] [PATCHv4 0/4] implement per-device cgroup blkio weight in qemu
v8: two new bug cleanup commits, move major()/minor() handling out of domain_conf.c into cgroup.c, and fix behavior when more than one device is listed, add a test for xml parsing. v7 was here (1-3 are already applied, 4-5 of that series are now 3-4): https://www.redhat.com/archives/libvir-list/2011-November/msg00317.html Eric Blake (2): API: prevent query of --live and --config at once qemu: fix blkiotune --live --config Hu Tao (2): blkiotune: add interface for blkiotune.device_weight blkiotune: add qemu support for blkiotune.device_weight docs/formatdomain.html.in | 29 +++- docs/schemas/domaincommon.rng | 26 ++- include/libvirt/libvirt.h.in | 10 + src/conf/domain_conf.c | 99 +- src/conf/domain_conf.h | 14 ++ src/libvirt.c | 36 +++- src/libvirt_private.syms |2 + src/qemu/qemu_cgroup.c | 20 ++ src/qemu/qemu_driver.c | 223 +++- src/util/cgroup.c | 51 + src/util/cgroup.h |4 + .../qemuxml2argv-blkiotune-device.args |4 + .../qemuxml2argv-blkiotune-device.xml | 36 +++ tests/qemuxml2argvtest.c |1 + tests/qemuxml2xmltest.c|1 + tools/virsh.c | 43 +++- tools/virsh.pod|8 +- 17 files changed, 576 insertions(+), 31 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml -- 1.7.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 1/4] API: prevent query of --live and --config at once
Drivers were inconsistent when presented both --live and --config
at once. For example, within qemu, getting memory parameters
favored live, while getting blkio tuning favored config. Some,
but not all, attempts to mix flags on query were filtered at
the virsh level, but we shouldn't have to duplicate efforts in
every client app. So, it is simpler to just enforce that the two
flags cannot both be used at once on query operations, which has
precedence, and which matches the documentation of
virDomainModificationImpact.
* src/libvirt.c (virDomainGetMemoryParameters)
(virDomainGetBlkioParameters)
(virDomainGetSchedulerParametersFlags, virDomainGetVcpuPinInfo):
Borrow sanity checking from virDomainGetVcpusFlags.
---
src/libvirt.c | 36
1 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 1518ed2..3a6e8e9 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3745,6 +3745,13 @@ virDomainGetMemoryParameters(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain-conn-driver, domain-conn,
VIR_DRV_FEATURE_TYPED_PARAM_STRING))
flags |= VIR_TYPED_PARAM_STRING_OKAY;
+
+/* At most one of these two flags should be set. */
+if ((flags VIR_DOMAIN_AFFECT_LIVE)
+(flags VIR_DOMAIN_AFFECT_CONFIG)) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
conn = domain-conn;
if (conn-driver-domainGetMemoryParameters) {
@@ -3870,6 +3877,13 @@ virDomainGetBlkioParameters(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain-conn-driver, domain-conn,
VIR_DRV_FEATURE_TYPED_PARAM_STRING))
flags |= VIR_TYPED_PARAM_STRING_OKAY;
+
+/* At most one of these two flags should be set. */
+if ((flags VIR_DOMAIN_AFFECT_LIVE)
+(flags VIR_DOMAIN_AFFECT_CONFIG)) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
conn = domain-conn;
if (conn-driver-domainGetBlkioParameters) {
@@ -6515,6 +6529,13 @@ virDomainGetSchedulerParametersFlags(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain-conn-driver, domain-conn,
VIR_DRV_FEATURE_TYPED_PARAM_STRING))
flags |= VIR_TYPED_PARAM_STRING_OKAY;
+
+/* At most one of these two flags should be set. */
+if ((flags VIR_DOMAIN_AFFECT_LIVE)
+(flags VIR_DOMAIN_AFFECT_CONFIG)) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
conn = domain-conn;
if (conn-driver-domainGetSchedulerParametersFlags) {
@@ -7914,7 +7935,8 @@ virDomainGetVcpusFlags(virDomainPtr domain, unsigned int
flags)
}
/* At most one of these two flags should be set. */
-if ((flags VIR_DOMAIN_AFFECT_LIVE) (flags
VIR_DOMAIN_AFFECT_CONFIG)) {
+if ((flags VIR_DOMAIN_AFFECT_LIVE)
+(flags VIR_DOMAIN_AFFECT_CONFIG)) {
virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
@@ -8011,7 +8033,7 @@ error:
* underlying virtualization system (Xen...).
* If maplen size, missing bytes are set to zero.
* If maplen size, failure code is returned.
- * @flags: bitwise-OR of virDomainModificationImpac
+ * @flags: bitwise-OR of virDomainModificationImpact
*
* Dynamically change the real CPUs which can be allocated to a virtual CPU.
* This function may require privileged access to the hypervisor.
@@ -8102,8 +8124,8 @@ error:
* -1 in case of failure.
*/
int
-virDomainGetVcpuPinInfo (virDomainPtr domain, int ncpumaps,
- unsigned char *cpumaps, int maplen, unsigned int
flags)
+virDomainGetVcpuPinInfo(virDomainPtr domain, int ncpumaps,
+unsigned char *cpumaps, int maplen, unsigned int flags)
{
virConnectPtr conn;
@@ -8124,6 +8146,12 @@ virDomainGetVcpuPinInfo (virDomainPtr domain, int
ncpumaps,
goto error;
}
+/* At most one of these two flags should be set. */
+if ((flags VIR_DOMAIN_AFFECT_LIVE)
+(flags VIR_DOMAIN_AFFECT_CONFIG)) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
conn = domain-conn;
if (conn-driver-domainGetVcpuPinInfo) {
--
1.7.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 3/4] blkiotune: add interface for blkiotune.device_weight
From: Hu Tao hu...@cn.fujitsu.com
This adds per-device weights to blkiotune. Note that the
cgroups implementation only supports weights per block device,
and not per-file within the device; hence this option must be
global to the domain definition rather than tied to individual
devices/disk entries:
domain ...
blkiotune
device
path/path/to/block/path
weight1000/weight
/device
/blkiotune
..
This patch also adds a parameter --device-weights to virsh command
blkiotune for setting/getting blkiotune.weight_device for any
hypervisor that supports it. All device entries under
blkiotune are concatenated into a single string attribute under
virDomain{Get,Set}BlkioParameters, named device_weight.
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
Signed-off-by: Eric Blake ebl...@redhat.com
---
docs/formatdomain.html.in | 29 +-
docs/schemas/domaincommon.rng | 26 -
include/libvirt/libvirt.h.in | 10 ++
src/conf/domain_conf.c | 99 +++-
src/conf/domain_conf.h | 14 +++
src/libvirt_private.syms |1 +
.../qemuxml2argv-blkiotune-device.xml | 36 +++
tools/virsh.c | 43 +++--
tools/virsh.pod|8 ++-
9 files changed, 245 insertions(+), 21 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index cbad196..99c5add 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -505,6 +505,14 @@
...
lt;blkiotunegt;
lt;weightgt;800lt;/weightgt;
+lt;devicegt;
+ lt;pathgt;/dev/sdalt;/pathgt;
+ lt;weightgt;1000lt;/weightgt;
+lt;/devicegt;
+lt;devicegt;
+ lt;pathgt;/dev/sdblt;/pathgt;
+ lt;weightgt;500lt;/weightgt;
+lt;/devicegt;
lt;/blkiotunegt;
...
lt;/domaingt;
@@ -514,10 +522,25 @@
dtcodeblkiotune/code/dt
dd The optional codeblkiotune/code element provides the ability
to tune Blkio cgroup tunable parameters for the domain. If this is
-omitted, it defaults to the OS provided defaults./dd
+omitted, it defaults to the OS provided
+defaults. span class=sinceSince 0.8.8/span/dd
dtcodeweight/code/dt
- dd The optional codeweight/code element is the I/O weight of the
-guest. The value should be in range [100, 1000]./dd
+ dd The optional codeweight/code element is the overall I/O
+weight of the guest. The value should be in the range [100,
+1000]./dd
+ dtcodedevice/code/dt
+ ddThe domain may have multiple codedevice/code elements
+that further tune the weights for each host block device in
+use by the domain. Note that
+multiple a href=#elementsDisksguest disks/a can share a
+single host block device, if they are backed by files within
+the same host file system, which is why this tuning parameter
+is at the global domain level rather than associated with each
+guest disk device. Each codedevice/code element has two
+mandatory sub-elements, codepath/code describing the
+absolute path of the device, and codeweight/code giving
+the relative weight of that device, in the range [100,
+1000]. span class=sinceSince 0.9.8/span/dd
/dl
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b6f858e..f776a51 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -322,12 +322,26 @@
!-- The Blkio cgroup related tunables would go in the blkiotune --
optional
element name=blkiotune
- !-- I/O weight the VM can use --
- optional
-element name=weight
- ref name=weight/
-/element
- /optional
+ interleave
+!-- I/O weight the VM can use --
+optional
+ element name=weight
+ref name=weight/
+ /element
+/optional
+zeroOrMore
+ element name=device
+interleave
+ element name=path
+ref name=absFilePath/
+ /element
+ element name=weight
+ref name=weight/
+ /element
+/interleave
+ /element
+/zeroOrMore
+ /interleave
/element
/optional
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2ab89f5..ff4f51b 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1236,6 +1236,16 @@ char *
virDomainGetSchedulerType(virDomainPtr domain,
#define VIR_DOMAIN_BLKIO_WEIGHT weight
+/**
+ *
[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 69 src/driver.h | 18 + src/libvirt.c| 142 ++ src/libvirt_public.syms |2 + 4 files changed, 231 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2ab89f5..f4988c4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1671,6 +1671,75 @@ int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* Block I/O throttling support */ + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it represents the total + * bytes per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC total_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * bytes per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC read_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the write + * bytes per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC write_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the total + * I/O operations per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC total_iops_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * I/O operations per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC read_iops_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC: + * Macro for the BlockIoTune tunable weight: it repersents the write + * I/O operations per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC write_iops_sec + +int +virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int nparams, +unsigned int flags); +int +virDomainGetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int *nparams, +unsigned int flags); + + /* * NUMA support */ diff --git a/src/driver.h b/src/driver.h index 4c14aaa..6ce3efc 100644 --- a/src/driver.h +++ b/src/driver.h @@ -740,6 +740,24 @@ typedef int (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef int +(*virDrvDomainSetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +typedef int +(*virDrvDomainGetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); + /** * _virDriver: diff --git a/src/libvirt.c b/src/libvirt.c index 1518ed2..32ad5db 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17149,3 +17149,145 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainSetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @params: Pointer to blkio parameter objects + * @nparams: Number of blkio parameters (this value can be the same or + * less than the number of parameters supported) + * @flags: An OR'ed set of virDomainModificationImpact + * + * Change all or a subset of the per-device block IO tunables. + * + * The @disk parameter is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding the target dev='...' + * attribute within //domain/devices/disk. (For example, xvda). + * + * Returns -1 in case of error, 0 in case of success. + */ +int virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int nparams, +unsigned int
[libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight
From: Hu Tao hu...@cn.fujitsu.com
Implement setting/getting per-device blkio weights in qemu,
using the cgroups blkio.weight_device tunable.
---
src/libvirt_private.syms |1 +
src/qemu/qemu_cgroup.c | 20 ++
src/qemu/qemu_driver.c | 218 +++-
src/util/cgroup.c | 51 +
src/util/cgroup.h |4 +
.../qemuxml2argv-blkiotune-device.args |4 +
tests/qemuxml2argvtest.c |1 +
tests/qemuxml2xmltest.c|1 +
8 files changed, 295 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d78fd53..3575fe0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -89,6 +89,7 @@ virCgroupKillRecursive;
virCgroupMounted;
virCgroupPathOfController;
virCgroupRemove;
+virCgroupSetBlkioDeviceWeight;
virCgroupSetBlkioWeight;
virCgroupSetCpuShares;
virCgroupSetCpuCfsPeriod;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2a10bd2..eda4c66 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -312,6 +312,26 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
}
+if (vm-def-blkio.ndevices) {
+if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+for (i = 0; i vm-def-blkio.ndevices; i++) {
+virBlkioDeviceWeightPtr dw = vm-def-blkio.devices[i];
+rc = virCgroupSetBlkioDeviceWeight(cgroup, dw-path,
+ dw-weight);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set io device weight
+ for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+}
+} else {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(Block I/O tuning is not available on this
host));
+}
+}
+
if (vm-def-mem.hard_limit != 0 ||
vm-def-mem.soft_limit != 0 ||
vm-def-mem.swap_hard_limit != 0) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b0ce115..43f4041 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -112,7 +112,7 @@
# define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */
#endif
-#define QEMU_NB_BLKIO_PARAM 1
+#define QEMU_NB_BLKIO_PARAM 2
static void processWatchdogEvent(void *data, void *opaque);
@@ -5885,6 +5885,105 @@ cleanup:
return ret;
}
+/* deviceWeightStr in the form of /device/path,weight,/device/path,weight
+ * for example, /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0,800
+ */
+static int
+parseBlkioWeightDeviceStr(char *deviceWeightStr,
+ virBlkioDeviceWeightPtr *dw, int *size,
+ virCgroupPtr cgroup)
+{
+char *temp;
+int ndevices = 0;
+int nsep = 0;
+int i;
+virBlkioDeviceWeightPtr result = NULL;
+
+temp = deviceWeightStr;
+while (temp) {
+temp = strchr(temp, ',');
+if (temp) {
+temp++;
+nsep++;
+}
+}
+
+/* A valid string must have even number of fields, hence an odd
+ * number of commas. */
+if (!(nsep 1))
+goto error;
+
+ndevices = (nsep + 1) / 2;
+
+if (VIR_ALLOC_N(result, ndevices) 0) {
+virReportOOMError();
+return -1;
+}
+
+i = 0;
+temp = deviceWeightStr;
+while (temp) {
+char *p = temp;
+
+/* device path */
+p = strchr(p, ',');
+if (!p)
+goto error;
+
+result[i].path = strndup(temp, p - temp);
+if (!result[i].path) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* weight */
+temp = p + 1;
+
+if (virStrToLong_ui(temp, p, 10, result[i].weight) 0)
+goto error;
+
+if (cgroup) {
+int rc = virCgroupSetBlkioDeviceWeight(cgroup,
+ result[i].path,
+ result[i].weight);
+if (rc 0) {
+virReportSystemError(-rc,
+ _(Unable to set io device weight
+ for path %s),
+ result[i].path);
+goto cleanup;
+}
+}
+
+/* 0-weight entries only affect cgroups, they don't stick in xml */
+if (result[i].weight)
+i++;
+else
+VIR_FREE(result[i].path);
+if (*p == '\0')
+break;
+else if (*p !=
[libvirt] [PATCHv4 2/4] qemu: fix blkiotune --live --config
Without this, 'virsh blkiotune --live --config --weight=n'
only affected live.
* src/qemu/qemu_driver.c (qemuDomainSetBlkioParameters): Allow
setting both configurations at once.
---
src/qemu/qemu_driver.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f4a18d..b0ce115 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5981,7 +5981,10 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
ret = -1;
}
}
-} else if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+}
+if (ret 0)
+goto cleanup;
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
/* Clang can't see that if we get here, persistentDef was set. */
sa_assert(persistentDef);
--
1.7.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] Doc: Add description and validation for blkdeviotune
Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- docs/formatdomain.html.in | 31 +++ docs/schemas/domaincommon.rng | 24 2 files changed, 55 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index cbad196..733062d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -893,6 +893,14 @@ lt;driver name=tap type=aio cache=default/gt; lt;source file='/var/lib/xen/images/fv0'/ startupPolicy='optional'gt; lt;target dev='hda' bus='ide'/gt; + lt;iotunegt; +lt;total_bytes_secgt;nlt;/total_bytes_secgt; +lt;read_bytes_secgt;nlt;/read_bytes_secgt; +lt;write_bytes_secgt;nlt;/write_bytes_secgt; +lt;total_iops_secgt;nlt;/total_iops_secgt; +lt;read_bytes_secgt;nlt;/read_bytes_secgt; +lt;write_bytes_secgt;nlt;/write_bytes_secgt; + lt;/iotunegt; lt;boot order='2'/gt; lt;encryption type='...'gt; ... @@ -1010,6 +1018,29 @@ span class=sinceSince 0.0.3; codebus/code attribute since 0.4.3; usb attribute value since after 0.4.4; sata attribute value since 0.9.7/span/dd + dtcodeiotune/code/dt + ddThe optional codeiotune/code element provides the ability +to set or get block I/O throttling for the device. Block I/O +throtting be implemented by qemu, is specified per-disk and can +vary across multiple disks./dd + dtcodetotal_bytes_sec/code/dt + ddThe optinal codetotal_bytes_sec/code element is the total throughput +limit in bytes per second./dd + dtcoderead_bytes_sec/code/dt + ddThe optinal coderead_bytes_sec/code element is the read throughput +limit in bytes per second./dd + dtcodewrite_bytes_sec/code/dt + ddThe optinal codewrite_bytes_sec/code element is the write throughput +limit in bytes per second./dd + dtcodetotal_iops_sec/code/dt + ddThe optional codetotal_iops_sec/code element is the total I/O operations +per second./dd + dtcoderead_iops_sec/code/dt + ddThe optional coderead_iops_sec/code element is the read I/O operations +per second./dd + dtcodewrite_iops_sec/code/dt + ddThe optional codewrite_iops_sec/code element is the write I/O operations +per second./dd dtcodedriver/code/dt dd The optional driver element allows specifying further details diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b6f858e..c6873a0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -698,6 +698,30 @@ optional ref name=snapshot/ /optional + optional +element name=iotune + choice +element name=total_bytes_sec + ref name=unsignedLongLong +/element +element name=read_bytes_sec + ref name=unsignedLongLong +/element +element name=write_bytes_sec + ref name=unsignedLongLong +/element +element name=total_iops_sec + ref name=unsignedLongLong +/element +element name=read_iops_sec + ref name=unsignedLongLong +/element +element name=write_iopw_sec + ref name=unsignedLongLong +/emement + /choice +/element + /optional choice group attribute name=type -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/8 v4] Summary on block IO throttle
Changes since V3
- Use virTypedParameterPtr instead of specific struct in libvirt pulic API.
- Relevant changes to remote driver, qemu driver, python support and virsh.
Changes since V2
- Implement the Python binding support for setting blkio throttling.
- Implement --current --live --config options support to unify the libvirt API.
- Add changes in docs and tests.
- Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange.
- Change the XML schema.
- API name to virDomain{Set, Get}BlockIoTune.
- Parameters changed to make them more self-explanatory.
- virsh command name to blkdeviotune.
- And other fixups.
Changes since V1
- Implement the support to get the block io throttling for
a device as read only connection - QMP/HMP.
- Split virDomainBlockIoThrottle into two separate functions
virDomainSetBlockIoThrottle - Set block I/O limits for a device
- Requires a connection in 'write' mode.
- Limits (info) structure passed as an input parameter
virDomainGetBlockIoThrottle - Get the current block I/O limits for a device
- Works on a read-only connection.
- Current limits are written to the output parameter (reply).
- And Other fixups suggested by Adam Litke, Daniel P. Berrange.
- For dynamically allocate the blkiothrottle struct, I will fix
it when implement --current --live --config options support.
Today libvirt supports the cgroups blkio-controller, which handles
proportional shares and throughput/iops limits on host block devices.
blkio-controller does not support network file systems (NFS) or other
QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are
not host block devices. QEMU I/O throttling works with all types of
drive and can be applied independently to each drive attached to
a guest and supports throughput/iops limits.
To help add QEMU I/O throttling support to libvirt, we plan to complete
it with add new API virDomain{Set, Get}BlockIoThrottle(), new command
'blkdeviotune'
and Python bindings.
Notes: Now all the planed features were implemented (#1#2 were implemented by
Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part
patches
have been accepted upstream and are expected to be part of the QEMU 1.1
release, git tree from Zhi Yong:
http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block
1) Enable the blkio throttling in xml when guest is starting up.
Add blkio throttling in xml as follows:
disk type='file' device='disk'
...
iotune
total_bytes_secnnn/total_bytes_sec
...
/iotune
...
/disk
2) Enable blkio throttling setting at guest running time.
virsh blkdeviotune domain device [--total_bytes_secnumber]
[--read_bytes_secnumber] \
[--write_bytes_secnumber] [--total_iops_secnumber]
[--read_iops_secnumber]
[--write_iops_secnumber]
3) The support to get the current block i/o throttling for a device - HMP/QMP.
virsh blkiothrottle domain device
total_bytes_sec:
read_bytes_sec:
write_bytes_sec:
total_iops_sec:
read_iops_sec:
write_iops_sec:
4) Python binding support for setting blkio throttling.
5) --current --live --config options support to unify the libvirt API.
virsh blkdeviotune domain device [--total_bytes_sec number]
[--read_bytes_sec number]
[--write_bytes_sec number] [--total_iops_sec number] [--read_iops_sec
number]
[--write_iops_sec number] [--config] [--live] [--current]
daemon/remote.c| 108 +++
docs/formatdomain.html.in | 31 ++
docs/schemas/domaincommon.rng | 24 ++
include/libvirt/libvirt.h.in | 70
python/generator.py|2 +
python/libvirt-override-api.xml| 16 +
python/libvirt-override.c | 179 +++
src/conf/domain_conf.c | 101 ++-
src/conf/domain_conf.h | 12 +
src/driver.h | 20 ++
src/libvirt.c | 145 +
src/libvirt_public.syms|2 +
src/qemu/qemu_command.c| 33 ++
src/qemu/qemu_driver.c | 338
src/qemu/qemu_monitor.c| 36 ++
src/qemu/qemu_monitor.h| 22 ++
src/qemu/qemu_monitor_json.c | 185 +++
src/qemu/qemu_monitor_json.h | 10 +
src/qemu/qemu_monitor_text.c | 164 ++
src/qemu/qemu_monitor_text.h | 10 +
src/remote/remote_driver.c | 96 ++
src/remote/remote_protocol.x | 26 ++-
src/remote_protocol-structs| 24 ++
src/util/xml.h |2
[libvirt] [PATCH 4/8] Support block I/O throtte in XML
Enable block I/O throttle for per-disk in XML.
Signed-off-by: Lei Li li...@linux.vnet.ibm.com
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
---
src/conf/domain_conf.c | 101 +-
src/conf/domain_conf.h | 12 ++
src/qemu/qemu_command.c | 33 +++
src/util/xml.h |2 +
4 files changed, 145 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 58f4d0f..a157b80 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr
virDomainDiskDefParseXML(virCapsPtr caps,
xmlNodePtr node,
virBitmapPtr bootMap,
- unsigned int flags)
+ unsigned int flags,
+ xmlXPathContextPtr ctxt)
{
virDomainDiskDefPtr def;
xmlNodePtr cur, child;
@@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps,
}
child = child-next;
}
+} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) {
+if
(virXPathULongLong(string(./devices/disk/iotune/total_bytes_sec),
+ ctxt,
def-blkdeviotune.total_bytes_sec) 0) {
+def-blkdeviotune.total_bytes_sec = 0;
+} else {
+def-blkdeviotune.mark = 1;
+}
+
+if
(virXPathULongLong(string(./devices/disk/iotune/read_bytes_sec),
+ ctxt,
def-blkdeviotune.read_bytes_sec) 0) {
+def-blkdeviotune.read_bytes_sec = 0;
+} else {
+def-blkdeviotune.mark = 1;
+}
+
+if
(virXPathULongLong(string(./devices/disk/iotune/write_bytes_sec),
+ ctxt,
def-blkdeviotune.write_bytes_sec) 0) {
+def-blkdeviotune.write_bytes_sec = 0;
+} else {
+def-blkdeviotune.mark = 1;
+}
+
+ if
(virXPathULongLong(string(./devices/disk/iotune/total_iops_sec),
+ ctxt,
def-blkdeviotune.total_iops_sec) 0) {
+def-blkdeviotune.total_iops_sec = 0;
+} else {
+def-blkdeviotune.mark = 1;
+}
+
+if
(virXPathULongLong(string(./devices/disk/iotune/read_iops_sec),
+ ctxt, def-blkdeviotune.read_iops_sec)
0) {
+def-blkdeviotune.read_iops_sec = 0;
+} else {
+def-blkdeviotune.mark = 1;
+}
+
+if
(virXPathULongLong(string(./devices/disk/iotune/write_iops_sec),
+ ctxt,
def-blkdeviotune.write_iops_sec) 0) {
+def-blkdeviotune.write_iops_sec = 0;
+} else {
+def-blkdeviotune.mark = 1;
+}
+
+if ((def-blkdeviotune.total_bytes_sec
def-blkdeviotune.read_bytes_sec)
+|| (def-blkdeviotune.total_bytes_sec
def-blkdeviotune.write_bytes_sec)) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(total and read/write bytes_sec
cannot be set at the same time));
+goto error;
+}
+
+if ((def-blkdeviotune.total_iops_sec
def-blkdeviotune.read_iops_sec)
+|| (def-blkdeviotune.total_iops_sec
def-blkdeviotune.write_iops_sec)) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(total and read/write iops_sec
cannot be set at the same time));
+goto error;
+}
} else if (xmlStrEqual(cur-name, BAD_CAST readonly)) {
def-readonly = 1;
} else if (xmlStrEqual(cur-name, BAD_CAST shareable)) {
@@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr
caps,
if (xmlStrEqual(node-name, BAD_CAST disk)) {
dev-type = VIR_DOMAIN_DEVICE_DISK;
if (!(dev-data.disk = virDomainDiskDefParseXML(caps, node,
-NULL, flags)))
+NULL, flags, NULL)))
goto error;
} else if (xmlStrEqual(node-name, BAD_CAST lease)) {
dev-type = VIR_DOMAIN_DEVICE_LEASE;
@@ -7076,7 +7133,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr
caps,
virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps,
nodes[i],
bootMap,
-flags);
+
[libvirt] [PATCH 8/8] Add tests for blkdeviotune
Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- .../qemuxml2argv-blkdeviotune.args |5 +++ .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml | 37 tests/qemuxml2argvtest.c |1 + tests/qemuxml2xmltest.c|1 + 4 files changed, 44 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args new file mode 100644 index 000..9db8aff --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor unix:/tmp/test-monitor, \ +server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial \ +none -parallel none -usb + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml new file mode 100644 index 000..0cabb90 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml @@ -0,0 +1,37 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory219100/memory + currentMemory219100/currentMemory + vcpu1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' snapshot='internal' + driver name='qemu' type='qcow2' cache='none'/ + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + iotune +total_bytes_sec5000/total_bytes_sec +total_iops_sec6000/total_iops_sec + address type='drive' controller='0' bus='0' unit='0'/ +/disk +disk type='block' device='cdrom' snapshot='no' + driver name='qemu' type='raw'/ + source dev='/dev/HostVG/QEMUGuest2'/ + target dev='hdc' bus='ide'/ + readonly/ + address type='drive' controller='0' bus='1' unit='0'/ +/disk +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d9a6e8d..1ebb950 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -585,6 +585,7 @@ mymain(void) DO_TEST(blkiotune, false, QEMU_CAPS_NAME); DO_TEST(cputune, false, QEMU_CAPS_NAME); DO_TEST(numatune-memory, false, NONE); +DO_TEST(blkdeviotune, false, QEMU_CAPS_NAME); DO_TEST(multifunction-pci-device, false, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3f37520..2e6b5c7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -191,6 +191,7 @@ mymain(void) DO_TEST(event_idx); DO_TEST(usb-redir); +DO_TEST(blkdeviotune); /* These tests generate different XML */ DO_TEST_DIFFERENT(balloon-device-auto); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
Support Block I/O Throttle setting and query to remote driver.
Signed-off-by: Lei Li li...@linux.vnet.ibm.com
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
---
daemon/remote.c | 109 ++
src/remote/remote_driver.c | 96 +
src/remote/remote_protocol.x | 26 ++-
src/remote_protocol-structs | 24 +
4 files changed, 254 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index aa3f768..227d36e 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1886,6 +1886,115 @@ cleanup:
return rv;
}
+static int
+remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client
ATTRIBUTE_UNUSED,
+ virNetMessagePtr hdr ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+
remote_domain_set_block_io_throttle_args *args)
+{
+virDomainPtr dom = NULL;
+int rv = -1;
+virTypedParameterPtr params = NULL;
+int nparams;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+if ((params = remoteDeserializeTypedParameters(args-params.params_val,
+ args-params.params_len,
+
REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX,
+ nparams)) == NULL)
+goto cleanup;
+
+rv = virDomainSetBlockIoTune(dom, args-disk, params, nparams,
args-flags);
+
+if (rv 0)
+goto cleanup;
+
+cleanup:
+if (rv 0)
+virNetMessageSaveError(rerr);
+if (dom)
+virDomainFree(dom);
+return rv;
+}
+
+static int
+remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client
ATTRIBUTE_UNUSED,
+ virNetMessagePtr hdr ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+
remote_domain_get_block_io_throttle_args *args,
+ remote_domain_get_block_io_throttle_ret
*ret)
+{
+virDomainPtr dom = NULL;
+int rv = -1;
+int i;
+virTypedParameterPtr params;
+int nparams = args-nparams;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (nparams REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(nparams too large));
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(params, nparams) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+if (virDomainGetBlockIoTune(dom, args-disk, params, nparams,
args-flags) 0)
+goto cleanup;
+
+/* In this case, we need to send back the number of parameters
+ * supported
+ */
+if (args-nparams == 0) {
+ret-nparams = nparams;
+goto success;
+}
+
+/* Serialise the block I/O throttle. */
+if (remoteSerializeTypedParameters(params, nparams,
+ ret-params.params_val,
+ ret-params.params_len,
+ args-flags) 0)
+goto cleanup;
+
+success:
+rv = 0;
+
+cleanup:
+if (rv 0) {
+virNetMessageSaveError(rerr);
+if (ret-params.params_val) {
+for (i = 0; i nparams; i++)
+VIR_FREE(ret-params.params_val[i].field);
+VIR_FREE(ret-params.params_val);
+}
+}
+if (dom)
+virDomainFree(dom);
+return rv;
+}
/*-*/
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 94fd3e7..fa2d2c7 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2178,6 +2178,100 @@ done:
return rv;
}
+static int remoteDomainSetBlockIoTune(virDomainPtr domain,
+ const char *disk,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
+{
+int rv = -1;
+remote_domain_set_block_io_throttle_args args;
+struct private_data *priv = domain-conn-privateData;
+
+remoteDriverLock(priv);
[libvirt] [PATCH 7/8] Support virDomain{Set, Get}BlockIoThrottle in the python API
Python support for both setting and getting block I/O throttle.
Signed-off-by: Lei Li li...@linux.vnet.ibm.com
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
---
python/generator.py |2 +
python/libvirt-override-api.xml | 16
python/libvirt-override.c | 178 +++
3 files changed, 196 insertions(+), 0 deletions(-)
diff --git a/python/generator.py b/python/generator.py
index 71afdb7..88c52b9 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -414,6 +414,8 @@ skip_impl = (
'virDomainGetBlockJobInfo',
'virDomainMigrateGetMaxSpeed',
'virDomainBlockStatsFlags',
+'virDomainSetBlockIoTune',
+'virDomainGetBlockIoTune',
)
qemu_skip_impl = (
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index ef02f34..a05da3c 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -375,5 +375,21 @@
arg name='flags' type='unsigned int' info='flags, currently unused,
pass 0.'/
return type='unsigned long' info='current max migration speed, or None
in case of error'/
/function
+function name='virDomainSetBlockIoTune' file='python'
+ infoChange the I/O throttle for a block device/info
+ arg name='dom' type='virDomainPtr' info='pointer to the domain'/
+ arg name='disk' type='const char *' info='disk name'/
+ arg name='params' type='virTypedParameterPtr' info='Pointer to blkio
throttle params object'/
+ arg name='flags' type='unsigned int' info='an ORapos;ed set of
virDomainModificationImpact'/
+ return type='int' info='0 in case of success, -1 in case of failure'/
+/function
+function name='virDomainGetBlockIoTune' file='python'
+ infoGet the I/O throttle a block device/info
+ arg name='dom' type='virDomainPtr' info='pointer to the domain'/
+ arg name='disk' type='const char *' info='disk name'/
+ arg name='params' type='virTypedParameterPtr' info='Pointer to blkio
throttle params object'/
+ arg name='flags' type='unsigned int' info='an ORapos;ed set of
virDomainModificationImpact'/
+ return type='int' info='0 in case of success, -1 in case of failure'/
+/function
/symbols
/api
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 1759bae..be76d87 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -3195,6 +3195,182 @@ LIBVIRT_END_ALLOW_THREADS;
return ret;
}
+static PyObject *
+libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED,
+PyObject *args)
+{
+virDomainPtr domain;
+PyObject *pyobj_domain, *pyinfo;
+const char *disk;
+unsigned int flags;
+virTypedParameterPtr params;
+int nparams = 0, i;
+int c_ret;
+
+if (!PyArg_ParseTuple(args, (char *)Ozi:virDomainSetBlockIoTune,
+ pyobj_domain, disk, pyinfo, flags))
+return(NULL);
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_ret = virDomainGetBlockIoTune(domain, disk, NULL, nparams, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (c_ret 0)
+return VIR_PY_INT_FAIL;
+
+if ((params = malloc(sizeof(*params)*nparams)) == NULL)
+return VIR_PY_INT_FAIL;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_ret = virDomainGetBlockIoTune(domain, disk, params, nparams, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (c_ret 0) {
+free(params);
+return VIR_PY_INT_FAIL;
+}
+
+/* convert to a Python tuple of long objects */
+for (i = 0; i nparams; i++) {
+PyObject *key, *val;
+key = libvirt_constcharPtrWrap(params[i].field);
+val = PyDict_GetItem(pyinfo, key);
+Py_DECREF(key);
+
+if (val == NULL)
+continue;
+
+switch (params[i].type) {
+case VIR_TYPED_PARAM_INT:
+params[i].value.i = (int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_UINT:
+params[i].value.ui = (unsigned int)PyInt_AS_LONG(val);
+break;
+
+case VIR_TYPED_PARAM_LLONG:
+params[i].value.l = (long long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_ULLONG:
+params[i].value.ul = (unsigned long long)PyLong_AsLongLong(val);
+break;
+
+case VIR_TYPED_PARAM_DOUBLE:
+params[i].value.d = (double)PyFloat_AsDouble(val);
+break;
+
+case VIR_TYPED_PARAM_BOOLEAN:
+{
+PyObject *hacktrue = PyBool_FromLong(1);
+params[i].value.b = hacktrue == val ? 1: 0;
+Py_DECREF(hacktrue);
+}
+break;
+
+default:
+free(params);
+return VIR_PY_INT_FAIL;
+}
+}
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_ret = virDomainSetMemoryParameters(domain, params, nparams, flags);
+
[libvirt] [PATCH 3/8] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
This patch implement the block I/O throttle setting and getting support to qemu
driver.
Signed-off-by: Lei Li li...@linux.vnet.ibm.com
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
---
src/qemu/qemu_driver.c | 340 ++
src/qemu/qemu_monitor.c | 37 +
src/qemu/qemu_monitor.h | 22 +++
src/qemu/qemu_monitor_json.c | 185 +++
src/qemu/qemu_monitor_json.h | 10 ++
src/qemu/qemu_monitor_text.c | 164
src/qemu/qemu_monitor_text.h | 10 ++
7 files changed, 768 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f4a18d..740d2a2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10775,6 +10775,344 @@ cleanup:
return ret;
}
+static int
+qemuDomainSetBlockIoTune(virDomainPtr dom,
+ const char *disk,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
+{
+struct qemud_driver *driver = dom-conn-privateData;
+virDomainObjPtr vm = NULL;
+qemuDomainObjPrivatePtr priv;
+virDomainDefPtr persistentDef = NULL;
+virDomainBlockIoTuneInfo info;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+const char *device = NULL;
+int ret = -1;
+int i;
+bool isActive;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+memset(info, 0, sizeof(info));
+
+qemuDriverLock(driver);
+virUUIDFormat(dom-uuid, uuidstr);
+vm = virDomainFindByUUID(driver-domains, dom-uuid);
+if (!vm) {
+qemuReportError(VIR_ERR_NO_DOMAIN,
+_(no domain with matching uuid '%s'), uuidstr);
+goto cleanup;
+}
+
+device = qemuDiskPathToAlias(vm, disk);
+if (!device) {
+goto cleanup;
+}
+
+if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
+goto cleanup;
+
+isActive = virDomainObjIsActive(vm);
+
+if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
+if (isActive)
+flags = VIR_DOMAIN_AFFECT_LIVE;
+else
+flags = VIR_DOMAIN_AFFECT_CONFIG;
+}
+
+if (!isActive (flags VIR_DOMAIN_AFFECT_LIVE)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(domain is not running));
+goto endjob;
+}
+
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+if (!vm-persistent) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(cannot change persistent config of a transient
domain));
+goto endjob;
+}
+if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm)))
+goto endjob;
+}
+
+for (i = 0; i nparams; i++) {
+virTypedParameterPtr param = params[i];
+
+if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) {
+
+info.total_bytes_sec = params[i].value.ul;
+} else if (STREQ(param-field,
VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) {
+info.read_bytes_sec = params[i].value.ul;
+} else if (STREQ(param-field,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) {
+info.write_bytes_sec = params[i].value.ul;
+} else if (STREQ(param-field,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) {
+info.total_iops_sec = params[i].value.ul;
+} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC))
{
+info.read_iops_sec = params[i].value.ul;
+} else if (STREQ(param-field,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) {
+info.write_iops_sec = params[i].value.ul;
+} else {
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(Unrecognized parameter));
+goto endjob;
+}
+}
+
+if ((info.total_bytes_sec info.read_bytes_sec) ||
+(info.total_bytes_sec info.write_bytes_sec)) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(total and read/write of bytes_sec cannot be set at
the same time));
+goto endjob;
+}
+
+if ((info.total_iops_sec info.read_iops_sec) ||
+(info.total_iops_sec info.write_iops_sec)) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(total and read/write of iops_sec cannot be set at
the same time));
+goto endjob;
+}
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+priv = vm-privateData;
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ret = qemuMonitorSetBlockIoThrottle(priv-mon, device, info, 1);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+}
+
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+sa_assert(persistentDef);
+int idx = virDomainDiskIndexByName(vm-def, disk, true);
+if (i 0)
+goto endjob;
+persistentDef-disks[idx]-blkdeviotune.total_bytes_sec =
[libvirt] [PATCH 5/8] Enable the blkdeviotune command in virsh
Support virsh command blkdeviotune. Can set or query a block disk
I/O throttle setting.
Signed-off-by: Lei Li li...@linux.vnet.ibm.com
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
---
tools/virsh.c | 240 +++
tools/virsh.pod | 23 +
2 files changed, 263 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 83dc3c7..9eec68e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -6074,6 +6074,245 @@ cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd)
}
/*
+ * blkdeviotune command
+ */
+static const vshCmdInfo info_blkdeviotune[] = {
+{help, N_(Set or query a block disk I/O throttle setting.)},
+{desc, N_(Set or query a block disk I/O throttle setting.\n \
+To query the block disk I/O throttle setting use the
following \
+ command: \n\n \
+virsh # blkdeviotune domain device)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_blkdeviotune[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)},
+{total_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limit
in bytes per second)},
+{read_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limit
in bytes per second)},
+{write_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limit
in bytes per second)},
+{total_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total I/O operations
limit per second)},
+{read_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read I/O operations
limit per second)},
+{write_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write I/O operations
limit per second)},
+{config, VSH_OT_BOOL, 0, N_(affect next boot)},
+{live, VSH_OT_BOOL, 0, N_(affect running domain)},
+{current, VSH_OT_BOOL, 0, N_(affect current domain)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+const char *name, *disk;
+unsigned long long total_bytes_sec = 0, read_bytes_sec = 0,
write_bytes_sec = 0;
+unsigned long long total_iops_sec = 0, read_iops_sec = 0, write_iops_sec =
0;
+int nparams = 0;
+virTypedParameterPtr params = NULL, temp = NULL;
+unsigned int flags = 0, i = 0;
+int rv = 0;
+int current = vshCommandOptBool(cmd, current);
+int config = vshCommandOptBool(cmd, config);
+int live = vshCommandOptBool(cmd, live);
+
+if (current) {
+if (live || config) {
+vshError(ctl, %s, _(--current must be specified exclusively));
+return false;
+}
+flags = VIR_DOMAIN_AFFECT_CURRENT;
+} else {
+if (config)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+}
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+goto out;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
+goto out;
+
+if (vshCommandOptString(cmd, device, disk) 0)
+goto out;
+
+if ((rv = vshCommandOptULongLong(cmd, total_bytes_sec,
total_bytes_sec)) 0) {
+vshError(ctl, %s,
+ _(Unable to parse integer parameter));
+goto out;
+} else if (rv 0) {
+nparams++;
+}
+
+if ((rv = vshCommandOptULongLong(cmd, read_bytes_sec, read_bytes_sec))
0) {
+vshError(ctl, %s,
+ _(Unable to parse integer parameter));
+goto out;
+} else if (rv 0) {
+nparams++;
+}
+
+if ((rv = vshCommandOptULongLong(cmd, write_bytes_sec,
write_bytes_sec)) 0) {
+vshError(ctl, %s,
+ _(Unable to parse integer parameter));
+goto out;
+} else if (rv 0) {
+nparams++;
+}
+
+if ((rv = vshCommandOptULongLong(cmd, total_iops_sec, total_iops_sec))
0) {
+vshError(ctl, %s,
+ _(Unable to parse integer parameter));
+goto out;
+} else if (rv 0) {
+nparams++;
+}
+
+if ((rv = vshCommandOptULongLong(cmd, read_iops_sec, read_iops_sec))
0) {
+vshError(ctl, %s,
+ _(Unable to parse integer parameter));
+goto out;
+} else if (rv 0) {
+nparams++;
+}
+
+if ((rv = vshCommandOptULongLong(cmd, write_iops_sec, write_iops_sec))
0) {
+vshError(ctl, %s,
+ _(Unable to parse integer parameter));
+goto out;
+} else if (rv 0) {
+nparams++;
+}
+
+if (nparams == 0) {
+
+if ((virDomainGetBlockIoTune(dom, disk, NULL, nparams, flags)) != 0) {
+vshError(ctl, %s,
+ _(Unable to get number of block I/O throttle
parameters));
+goto out;
+}
+
+if (nparams == 0) {
+virDomainFree(dom);
+return true;
+}
+
+params = vshCalloc(ctl, nparams, sizeof(*params));
+
+if
Re: [libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
On 11/14/2011 09:33 PM, Lei Li wrote:
This patch add new pulic API virDomainSetBlockIoTune and
virDomainGetBlockIoTune.
+/**
+ * virDomainSetBlockIoTune:
+ * @dom: pointer to domain object
+ * @disk: Fully-qualified disk name
Hmm. Fully-qualified name here,
+ * @params: Pointer to blkio parameter objects
+ * @nparams: Number of blkio parameters (this value can be the same or
+ * less than the number of parameters supported)
+ * @flags: An OR'ed set of virDomainModificationImpact
+ *
+ * Change all or a subset of the per-device block IO tunables.
+ *
+ * The @disk parameter is the name of the block device. Get this
+ * by calling virDomainGetXMLDesc and finding the target dev='...'
+ * attribute within //domain/devices/disk. (For example, xvda).
but device shorthand here.
We probably ought to accept both forms, just as we do with
virDomainBlockStats, as of commit 89b6284f (oh, and that means that I
ought to update virDomainBlockStats to document the same thing) - which
means this can be a separate cleanup to make libvirt.c docs consistent
as well as teaching the new API to use the domain_conf.c
virDomainDiskIndexByName method to allow both forms. I can get to that
after finishing my review of this series, if you don't beat me.
+ * Get all block IO tunable parameters for a given device. On input,
+ * @nparams gives the size of the @params array; on output, @nparams
+ * gives how many slots were filled with parameter information, which
+ * might be less but will not exceed the input value.
+ *
+ * As a special case, calling with @params as NULL and @nparams as 0 on
+ * input will cause @nparams on output to contain the number of parameters
+ * supported by the hypervisor. The caller should then allocate @params
+ * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API
+ * agiain.
Trailing space (running 'make syntax-check' will gripe).
s/agiain/again/
Also, probably worth mentioning that the value of @nparams may be
disk-specific, so the user is best off reprobing an appropriate nparams
for each disk rather than assuming the parameters from 1 disk apply to
all others.
+++ b/src/libvirt_public.syms
@@ -496,6 +496,8 @@ LIBVIRT_0.9.7 {
virDomainSnapshotGetParent;
virDomainSnapshotListChildrenNames;
virDomainSnapshotNumChildren;
+virDomainSetBlockIoTune;
+virDomainGetBlockIoTune;
} LIBVIRT_0.9.5;
This needs to be in a new section LIBVIRT_0.9.8 based off of
LIBVIRT_0.9.5 (we aren't changing 0.9.7 at this point).
The problems I mentioned are minor enough that I don't mind fixing them
prior to pushing, if the rest of the series looks sane; but if later
review finds more problems later in the series, we may need a v5 series
instead. I'll see about reviewing the rest of the series this week
(I've also got quite a few nwfilter patches backed up in my todo queue).
--
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] [PATCHv4 0/4] implement per-device cgroup blkio weight in qemu
On 11/14/2011 09:29 PM, Eric Blake wrote: Oops - subject line glitch - this is v8, not v4. v8: two new bug cleanup commits, move major()/minor() handling out of domain_conf.c into cgroup.c, and fix behavior when more than one device is listed, add a test for xml parsing. v7 was here (1-3 are already applied, 4-5 of that series are now 3-4): https://www.redhat.com/archives/libvir-list/2011-November/msg00317.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
Re: [libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
On 11/14/2011 09:33 PM, Lei Li wrote:
Support Block I/O Throttle setting and query to remote driver.
Signed-off-by: Lei Li li...@linux.vnet.ibm.com
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
+
+struct remote_domain_get_block_io_throttle_args {
+remote_nonnull_domain dom;
+remote_nonnull_string disk;
+int nparams;
+unsigned int flags;
+};
Hmm, just wondering - if we use 'remote_string disk', then we can allow
NULL for disk and params as a way to populate nparams as the maximum
possible for all types of disks in a given domain, rather than returning
a possibly-varying nparams specific to each disk. Might be worth a
tweak to the libvirt.c documentation to allow NULL disk as a special case.
@@ -2564,7 +2586,9 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen
priority:high */
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen
autogen priority:high */
REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248, /* skipgen skipgen */
-REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249 /* skipgen skipgen */
+REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249, /* skipgen skipgen */
+REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 250, /* skipgen skipgen */
+REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE = 251 /* skipgen skipgen */
/*
* Notice how the entries are grouped in sets of 10 ?
Oops - you skipped right over the comment here, and forgot your blank
line :)
--
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 1/8] Add new API virDomain{Set, Get}BlockIoTune
On 11/15/2011 12:46 PM, Eric Blake wrote:
On 11/14/2011 09:33 PM, Lei Li wrote:
This patch add new pulic API virDomainSetBlockIoTune and
virDomainGetBlockIoTune.
+/**
+ * virDomainSetBlockIoTune:
+ * @dom: pointer to domain object
+ * @disk: Fully-qualified disk name
Hmm. Fully-qualified name here,
+ * @params: Pointer to blkio parameter objects
+ * @nparams: Number of blkio parameters (this value can be the same or
+ * less than the number of parameters supported)
+ * @flags: An OR'ed set of virDomainModificationImpact
+ *
+ * Change all or a subset of the per-device block IO tunables.
+ *
+ * The @disk parameter is the name of the block device. Get this
+ * by calling virDomainGetXMLDesc and finding thetarget dev='...'
+ * attribute within //domain/devices/disk. (For example, xvda).
but device shorthand here.
We probably ought to accept both forms, just as we do with
virDomainBlockStats, as of commit 89b6284f (oh, and that means that I
ought to update virDomainBlockStats to document the same thing) - which
means this can be a separate cleanup to make libvirt.c docs consistent
as well as teaching the new API to use the domain_conf.c
virDomainDiskIndexByName method to allow both forms. I can get to that
after finishing my review of this series, if you don't beat me.
Yes, both fully-qualified and shorthand name can all work.
+ * Get all block IO tunable parameters for a given device. On input,
+ * @nparams gives the size of the @params array; on output, @nparams
+ * gives how many slots were filled with parameter information, which
+ * might be less but will not exceed the input value.
+ *
+ * As a special case, calling with @params as NULL and @nparams as 0 on
+ * input will cause @nparams on output to contain the number of parameters
+ * supported by the hypervisor. The caller should then allocate @params
+ * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API
+ * agiain.
Trailing space (running 'make syntax-check' will gripe).
Thanks..:)
s/agiain/again/
Also, probably worth mentioning that the value of @nparams may be
disk-specific, so the user is best off reprobing an appropriate nparams
for each disk rather than assuming the parameters from 1 disk apply to
all others.
The QEMU Block I/O Throttling support these whole 6 parameters, so I think
display all the fields might make sense.
+++ b/src/libvirt_public.syms
@@ -496,6 +496,8 @@ LIBVIRT_0.9.7 {
virDomainSnapshotGetParent;
virDomainSnapshotListChildrenNames;
virDomainSnapshotNumChildren;
+virDomainSetBlockIoTune;
+virDomainGetBlockIoTune;
} LIBVIRT_0.9.5;
This needs to be in a new section LIBVIRT_0.9.8 based off of
LIBVIRT_0.9.5 (we aren't changing 0.9.7 at this point).
The problems I mentioned are minor enough that I don't mind fixing them
prior to pushing, if the rest of the series looks sane; but if later
review finds more problems later in the series, we may need a v5 series
instead. I'll see about reviewing the rest of the series this week
(I've also got quite a few nwfilter patches backed up in my todo queue).
OK. :) Thank you.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Lei
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 3/4] blkiotune: add interface for blkiotune.device_weight
On Mon, Nov 14, 2011 at 09:30:01PM -0700, Eric Blake wrote:
From: Hu Tao hu...@cn.fujitsu.com
This adds per-device weights to blkiotune. Note that the
cgroups implementation only supports weights per block device,
and not per-file within the device; hence this option must be
global to the domain definition rather than tied to individual
devices/disk entries:
domain ...
blkiotune
device
path/path/to/block/path
weight1000/weight
/device
/blkiotune
..
This patch also adds a parameter --device-weights to virsh command
blkiotune for setting/getting blkiotune.weight_device for any
hypervisor that supports it. All device entries under
blkiotune are concatenated into a single string attribute under
virDomain{Get,Set}BlkioParameters, named device_weight.
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
Signed-off-by: Eric Blake ebl...@redhat.com
---
docs/formatdomain.html.in | 29 +-
docs/schemas/domaincommon.rng | 26 -
include/libvirt/libvirt.h.in | 10 ++
src/conf/domain_conf.c | 99
+++-
src/conf/domain_conf.h | 14 +++
src/libvirt_private.syms |1 +
.../qemuxml2argv-blkiotune-device.xml | 36 +++
tools/virsh.c | 43 +++--
tools/virsh.pod|8 ++-
9 files changed, 245 insertions(+), 21 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index cbad196..99c5add 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -505,6 +505,14 @@
...
lt;blkiotunegt;
lt;weightgt;800lt;/weightgt;
+lt;devicegt;
+ lt;pathgt;/dev/sdalt;/pathgt;
+ lt;weightgt;1000lt;/weightgt;
+lt;/devicegt;
+lt;devicegt;
+ lt;pathgt;/dev/sdblt;/pathgt;
+ lt;weightgt;500lt;/weightgt;
+lt;/devicegt;
lt;/blkiotunegt;
...
lt;/domaingt;
@@ -514,10 +522,25 @@
dtcodeblkiotune/code/dt
dd The optional codeblkiotune/code element provides the ability
to tune Blkio cgroup tunable parameters for the domain. If this is
-omitted, it defaults to the OS provided defaults./dd
+omitted, it defaults to the OS provided
+defaults. span class=sinceSince 0.8.8/span/dd
dtcodeweight/code/dt
- dd The optional codeweight/code element is the I/O weight of the
-guest. The value should be in range [100, 1000]./dd
+ dd The optional codeweight/code element is the overall I/O
+weight of the guest. The value should be in the range [100,
+1000]./dd
+ dtcodedevice/code/dt
+ ddThe domain may have multiple codedevice/code elements
+that further tune the weights for each host block device in
+use by the domain. Note that
+multiple a href=#elementsDisksguest disks/a can share a
+single host block device, if they are backed by files within
+the same host file system, which is why this tuning parameter
+is at the global domain level rather than associated with each
+guest disk device. Each codedevice/code element has two
+mandatory sub-elements, codepath/code describing the
+absolute path of the device, and codeweight/code giving
+the relative weight of that device, in the range [100,
+1000]. span class=sinceSince 0.9.8/span/dd
/dl
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b6f858e..f776a51 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -322,12 +322,26 @@
!-- The Blkio cgroup related tunables would go in the blkiotune --
optional
element name=blkiotune
- !-- I/O weight the VM can use --
- optional
-element name=weight
- ref name=weight/
-/element
- /optional
+ interleave
+!-- I/O weight the VM can use --
+optional
+ element name=weight
+ref name=weight/
+ /element
+/optional
+zeroOrMore
+ element name=device
+interleave
+ element name=path
+ref name=absFilePath/
+ /element
+ element name=weight
+ref name=weight/
+ /element
+/interleave
+ /element
+/zeroOrMore
+ /interleave
/element
/optional
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2ab89f5..ff4f51b 100644
--- a/include/libvirt/libvirt.h.in
+++
Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight
On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
From: Hu Tao hu...@cn.fujitsu.com
Implement setting/getting per-device blkio weights in qemu,
using the cgroups blkio.weight_device tunable.
---
src/libvirt_private.syms |1 +
src/qemu/qemu_cgroup.c | 20 ++
src/qemu/qemu_driver.c | 218
+++-
src/util/cgroup.c | 51 +
src/util/cgroup.h |4 +
.../qemuxml2argv-blkiotune-device.args |4 +
tests/qemuxml2argvtest.c |1 +
tests/qemuxml2xmltest.c|1 +
8 files changed, 295 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d78fd53..3575fe0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -89,6 +89,7 @@ virCgroupKillRecursive;
virCgroupMounted;
virCgroupPathOfController;
virCgroupRemove;
+virCgroupSetBlkioDeviceWeight;
virCgroupSetBlkioWeight;
virCgroupSetCpuShares;
virCgroupSetCpuCfsPeriod;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2a10bd2..eda4c66 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -312,6 +312,26 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
}
+if (vm-def-blkio.ndevices) {
+if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO))
{
+for (i = 0; i vm-def-blkio.ndevices; i++) {
+virBlkioDeviceWeightPtr dw = vm-def-blkio.devices[i];
+rc = virCgroupSetBlkioDeviceWeight(cgroup, dw-path,
+ dw-weight);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set io device weight
+ for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+}
+} else {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(Block I/O tuning is not available on this
host));
+}
+}
+
if (vm-def-mem.hard_limit != 0 ||
vm-def-mem.soft_limit != 0 ||
vm-def-mem.swap_hard_limit != 0) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b0ce115..43f4041 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -112,7 +112,7 @@
# define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */
#endif
-#define QEMU_NB_BLKIO_PARAM 1
+#define QEMU_NB_BLKIO_PARAM 2
static void processWatchdogEvent(void *data, void *opaque);
@@ -5885,6 +5885,105 @@ cleanup:
return ret;
}
+/* deviceWeightStr in the form of /device/path,weight,/device/path,weight
+ * for example, /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0,800
+ */
+static int
+parseBlkioWeightDeviceStr(char *deviceWeightStr,
+ virBlkioDeviceWeightPtr *dw, int *size,
+ virCgroupPtr cgroup)
+{
+char *temp;
+int ndevices = 0;
+int nsep = 0;
+int i;
+virBlkioDeviceWeightPtr result = NULL;
+
+temp = deviceWeightStr;
+while (temp) {
+temp = strchr(temp, ',');
+if (temp) {
+temp++;
+nsep++;
+}
+}
+
+/* A valid string must have even number of fields, hence an odd
+ * number of commas. */
+if (!(nsep 1))
+goto error;
+
+ndevices = (nsep + 1) / 2;
+
+if (VIR_ALLOC_N(result, ndevices) 0) {
+virReportOOMError();
+return -1;
+}
+
+i = 0;
+temp = deviceWeightStr;
+while (temp) {
+char *p = temp;
+
+/* device path */
+p = strchr(p, ',');
+if (!p)
+goto error;
+
+result[i].path = strndup(temp, p - temp);
+if (!result[i].path) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* weight */
+temp = p + 1;
+
+if (virStrToLong_ui(temp, p, 10, result[i].weight) 0)
+goto error;
+
+if (cgroup) {
+int rc = virCgroupSetBlkioDeviceWeight(cgroup,
+ result[i].path,
+ result[i].weight);
Does more than the function name says. Would it be better to just do the
parsing here, and set the cgroup values after parsing(see my comment to
patch 3 for filtering out 0-weight when writing to xml):
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 43f4041..275d5c8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@
