Re: [libvirt] rbd storage pool support for libvirt
On Thu, Nov 18, 2010 at 5:13 PM, Sage Weil s...@newdream.net wrote: On Thu, 18 Nov 2010, Daniel P. Berrange wrote: On Wed, Nov 17, 2010 at 04:33:07PM -0800, Josh Durgin wrote: Hi Daniel, On 11/08/2010 05:16 AM, Daniel P. Berrange wrote: In any case, before someone goes off and implements something, does this look like the right general approach to adding rbd support to libvirt? I think this looks reasonable. I'd be inclined to get the storage pool stuff working with the kernel RBD driver UDEV rules for stable path names, since that avoids needing to make any changes to guest XML format. Support for QEMU with the native librados CEPH driver could be added as a second patch. Okay, that sounds reasonable. Supporting the QEMU librados driver is definitely something we want to target, though, and seems to be route that more users are interested in. Is defining the XML syntax for a guest VM something we can discuss now as well? (BTW this is biting NBD users too. Presumably the guest VM XML should look similar? And also Sheepdog storage volumes. To define a syntax for all these we need to determine what configuration metadata is required at a per-VM level for each of them. Then try and decide how to represent that in the guest XML. It looks like at a VM level we'd need a hostname, port number and a volume name (or path). It looks like that's what Sheepdog needs from the patch that was submitted earlier today. For RBD, we would want to allow multiple hosts, and specify the pool and image name when the QEMU librados driver is used, e.g.: disk type=rbd device=disk driver name=qemu type=raw / source vdi=image_name pool=pool_name host name=mon1.example.org port=6000 host name=mon2.example.org port=6000 host name=mon3.example.org port=6000 /source target dev=vda bus=virtio / /disk Does this seem like a reasonable format for the VM XML? Any suggestions? I'm basically wondering whether we should be going for separate types for each of NBD, RBD Sheepdog, as per your proposal the sheepdog one earlier today. Or type to merge them into one type 'nework' which covers any kind of network block device, and list a protocol on the source element, eg disk type=network device=disk driver name=qemu type=raw / source protocol='rbd|sheepdog|nbd' name=...some image identifier... host name=mon1.example.org port=6000 host name=mon2.example.org port=6000 host name=mon3.example.org port=6000 /source target dev=vda bus=virtio / /disk That would work... One thing that I think should be considered, though, is that both RBD and NBD can be used for non-qemu instances by mapping a regular block device via the host's kernel. And in that case, there's some sysfs-fu (at least in the rbd case; I'm not familiar with how the nbd client works) required to set up/tear down the block device. An nbd block device is attached using the nbd-client(1) userspace tool: $ nbd-client my-server 1234 /dev/nbd0 # host port nbd-device That program will open the socket, grab /dev/nbd0, and poke it with a few ioctls so the kernel has the socket and can take it from there. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] rbd storage pool support for libvirt
On Fri, Nov 19, 2010 at 09:27:40AM +, Stefan Hajnoczi wrote: On Thu, Nov 18, 2010 at 5:13 PM, Sage Weil s...@newdream.net wrote: On Thu, 18 Nov 2010, Daniel P. Berrange wrote: On Wed, Nov 17, 2010 at 04:33:07PM -0800, Josh Durgin wrote: Hi Daniel, On 11/08/2010 05:16 AM, Daniel P. Berrange wrote: In any case, before someone goes off and implements something, does this look like the right general approach to adding rbd support to libvirt? I think this looks reasonable. I'd be inclined to get the storage pool stuff working with the kernel RBD driver UDEV rules for stable path names, since that avoids needing to make any changes to guest XML format. Support for QEMU with the native librados CEPH driver could be added as a second patch. Okay, that sounds reasonable. Supporting the QEMU librados driver is definitely something we want to target, though, and seems to be route that more users are interested in. Is defining the XML syntax for a guest VM something we can discuss now as well? (BTW this is biting NBD users too. Presumably the guest VM XML should look similar? And also Sheepdog storage volumes. To define a syntax for all these we need to determine what configuration metadata is required at a per-VM level for each of them. Then try and decide how to represent that in the guest XML. It looks like at a VM level we'd need a hostname, port number and a volume name (or path). It looks like that's what Sheepdog needs from the patch that was submitted earlier today. For RBD, we would want to allow multiple hosts, and specify the pool and image name when the QEMU librados driver is used, e.g.: disk type=rbd device=disk driver name=qemu type=raw / source vdi=image_name pool=pool_name host name=mon1.example.org port=6000 host name=mon2.example.org port=6000 host name=mon3.example.org port=6000 /source target dev=vda bus=virtio / /disk Does this seem like a reasonable format for the VM XML? Any suggestions? I'm basically wondering whether we should be going for separate types for each of NBD, RBD Sheepdog, as per your proposal the sheepdog one earlier today. Or type to merge them into one type 'nework' which covers any kind of network block device, and list a protocol on the source element, eg disk type=network device=disk driver name=qemu type=raw / source protocol='rbd|sheepdog|nbd' name=...some image identifier... host name=mon1.example.org port=6000 host name=mon2.example.org port=6000 host name=mon3.example.org port=6000 /source target dev=vda bus=virtio / /disk That would work... One thing that I think should be considered, though, is that both RBD and NBD can be used for non-qemu instances by mapping a regular block device via the host's kernel. And in that case, there's some sysfs-fu (at least in the rbd case; I'm not familiar with how the nbd client works) required to set up/tear down the block device. An nbd block device is attached using the nbd-client(1) userspace tool: $ nbd-client my-server 1234 /dev/nbd0 # host port nbd-device That program will open the socket, grab /dev/nbd0, and poke it with a few ioctls so the kernel has the socket and can take it from there. We don't need to worry about this for libvirt/QEMU. Since QEMU has native NBD client support there's no need to do anything with nbd client tools to setup the device for use with a VM. Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] allow '#' as valid character for domain name
* docs/schemas/domain.rng --- docs/schemas/domain.rng |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index bbbc846..815134d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -2003,7 +2003,7 @@ /define define name=domainName data type=string - param name=pattern[A-Za-z0-9_\.\+\-amp;:/]+/param + param name=pattern[A-Za-z0-9_\.\+\-\\#amp;:/]+/param /data /define define name=diskSerial -- 1.7.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] rbd storage pool support for libvirt
On Fri, Nov 19, 2010 at 9:50 AM, Daniel P. Berrange berra...@redhat.com wrote: On Fri, Nov 19, 2010 at 09:27:40AM +, Stefan Hajnoczi wrote: On Thu, Nov 18, 2010 at 5:13 PM, Sage Weil s...@newdream.net wrote: On Thu, 18 Nov 2010, Daniel P. Berrange wrote: On Wed, Nov 17, 2010 at 04:33:07PM -0800, Josh Durgin wrote: Hi Daniel, On 11/08/2010 05:16 AM, Daniel P. Berrange wrote: In any case, before someone goes off and implements something, does this look like the right general approach to adding rbd support to libvirt? I think this looks reasonable. I'd be inclined to get the storage pool stuff working with the kernel RBD driver UDEV rules for stable path names, since that avoids needing to make any changes to guest XML format. Support for QEMU with the native librados CEPH driver could be added as a second patch. Okay, that sounds reasonable. Supporting the QEMU librados driver is definitely something we want to target, though, and seems to be route that more users are interested in. Is defining the XML syntax for a guest VM something we can discuss now as well? (BTW this is biting NBD users too. Presumably the guest VM XML should look similar? And also Sheepdog storage volumes. To define a syntax for all these we need to determine what configuration metadata is required at a per-VM level for each of them. Then try and decide how to represent that in the guest XML. It looks like at a VM level we'd need a hostname, port number and a volume name (or path). It looks like that's what Sheepdog needs from the patch that was submitted earlier today. For RBD, we would want to allow multiple hosts, and specify the pool and image name when the QEMU librados driver is used, e.g.: disk type=rbd device=disk driver name=qemu type=raw / source vdi=image_name pool=pool_name host name=mon1.example.org port=6000 host name=mon2.example.org port=6000 host name=mon3.example.org port=6000 /source target dev=vda bus=virtio / /disk Does this seem like a reasonable format for the VM XML? Any suggestions? I'm basically wondering whether we should be going for separate types for each of NBD, RBD Sheepdog, as per your proposal the sheepdog one earlier today. Or type to merge them into one type 'nework' which covers any kind of network block device, and list a protocol on the source element, eg disk type=network device=disk driver name=qemu type=raw / source protocol='rbd|sheepdog|nbd' name=...some image identifier... host name=mon1.example.org port=6000 host name=mon2.example.org port=6000 host name=mon3.example.org port=6000 /source target dev=vda bus=virtio / /disk That would work... One thing that I think should be considered, though, is that both RBD and NBD can be used for non-qemu instances by mapping a regular block device via the host's kernel. And in that case, there's some sysfs-fu (at least in the rbd case; I'm not familiar with how the nbd client works) required to set up/tear down the block device. An nbd block device is attached using the nbd-client(1) userspace tool: $ nbd-client my-server 1234 /dev/nbd0 # host port nbd-device That program will open the socket, grab /dev/nbd0, and poke it with a few ioctls so the kernel has the socket and can take it from there. We don't need to worry about this for libvirt/QEMU. Since QEMU has native NBD client support there's no need to do anything with nbd client tools to setup the device for use with a VM. I agree it's easier to use the built-in NBD support. Just wanted to provide the background on how NBD client works when using the kernel implementation. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] PHYP: Adding network management support
This is a series of 3 patches to add network management support for pHyp driver. Eduardo Otubo (3): PHYP: Separating UUID functions in another file PHYP: Adding basic network functions PHYP: create, destroy and other network functions po/POTFILES.in |1 + src/Makefile.am|3 +- src/phyp/phyp_driver.c | 1066 +++- src/phyp/phyp_driver.h | 43 ++- src/phyp/phyp_uuid.c | 834 + src/phyp/phyp_uuid.h | 44 ++ 6 files changed, 1520 insertions(+), 471 deletions(-) create mode 100644 src/phyp/phyp_uuid.c create mode 100644 src/phyp/phyp_uuid.h -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] PHYP: Separating UUID functions in another file
I am moving all the UUID handling functions to phyp_uuid.[ch] files in
order not to bloat the main files phyp_driver.[ch] too much. Doing this
for two reasons:
1) Network management in pHyp does not have a UUID.
2) Need to create another set of functions to manage it.
I also modified some functions to support two types of execution:
DOMAIN and NET, so I can re-use the base common functions.
---
po/POTFILES.in |1 +
src/Makefile.am|3 +-
src/phyp/phyp_driver.c | 464 +-
src/phyp/phyp_driver.h | 41 +++
src/phyp/phyp_uuid.c | 657
src/phyp/phyp_uuid.h | 36 +++
6 files changed, 742 insertions(+), 460 deletions(-)
create mode 100644 src/phyp/phyp_uuid.c
create mode 100644 src/phyp/phyp_uuid.h
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 2820ac1..e892d0b 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -50,6 +50,7 @@ src/opennebula/one_driver.c
src/openvz/openvz_conf.c
src/openvz/openvz_driver.c
src/phyp/phyp_driver.c
+src/phyp/phyp_uuid.c
src/qemu/qemu_bridge_filter.c
src/qemu/qemu_conf.c
src/qemu/qemu_driver.c
diff --git a/src/Makefile.am b/src/Makefile.am
index a9a1986..608b913 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -248,7 +248,8 @@ SECURITY_DRIVER_APPARMOR_HELPER_SOURCES =
\
security/virt-aa-helper.c
PHYP_DRIVER_SOURCES = \
- phyp/phyp_driver.c phyp/phyp_driver.h
+ phyp/phyp_driver.c phyp/phyp_driver.h \
+ phyp/phyp_uuid.c phyp/phyp_uuid.h
OPENVZ_DRIVER_SOURCES =\
openvz/openvz_conf.c openvz/openvz_conf.h \
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 4c723a2..6f3f49d 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -59,8 +59,10 @@
#include storage_conf.h
#include nodeinfo.h
#include files.h
+#include network_conf.h
#include phyp_driver.h
+#include phyp_uuid.h
#define VIR_FROM_THIS VIR_FROM_PHYP
@@ -75,7 +77,7 @@
static unsigned const int HMC = 0;
static unsigned const int IVM = 127;
-static int
+int
waitsocket(int socket_fd, LIBSSH2_SESSION * session)
{
struct timeval timeout;
@@ -307,7 +309,7 @@ phypCapsInit(void)
* 1 - Not Activated
* * - All
* */
-static int
+int
phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)
{
ConnectionData *connection_data = conn-networkPrivateData;
@@ -371,7 +373,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)
* type: 0 - Running
* 1 - all
* */
-static int
+int
phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
unsigned int type)
{
@@ -432,462 +434,6 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int
nids,
}
static int
-phypUUIDTable_WriteFile(virConnectPtr conn)
-{
-phyp_driverPtr phyp_driver = conn-privateData;
-uuid_tablePtr uuid_table = phyp_driver-uuid_table;
-unsigned int i = 0;
-int fd = -1;
-char local_file[] = ./uuid_table;
-
-if ((fd = creat(local_file, 0755)) == -1)
-goto err;
-
-for (i = 0; i uuid_table-nlpars; i++) {
-if (safewrite(fd, uuid_table-lpars[i]-id,
- sizeof(uuid_table-lpars[i]-id)) !=
-sizeof(uuid_table-lpars[i]-id)) {
-VIR_ERROR0(_(Unable to write information to local file.));
-goto err;
-}
-
-if (safewrite(fd, uuid_table-lpars[i]-uuid, VIR_UUID_BUFLEN) !=
-VIR_UUID_BUFLEN) {
-VIR_ERROR0(_(Unable to write information to local file.));
-goto err;
-}
-}
-
-if (VIR_CLOSE(fd) 0) {
-virReportSystemError(errno, _(Could not close %s),
- local_file);
-goto err;
-}
-return 0;
-
- err:
-VIR_FORCE_CLOSE(fd);
-return -1;
-}
-
-static int
-phypUUIDTable_Push(virConnectPtr conn)
-{
-ConnectionData *connection_data = conn-networkPrivateData;
-LIBSSH2_SESSION *session = connection_data-session;
-LIBSSH2_CHANNEL *channel = NULL;
-virBuffer username = VIR_BUFFER_INITIALIZER;
-struct stat local_fileinfo;
-char buffer[1024];
-int rc = 0;
-FILE *fd;
-size_t nread, sent;
-char *ptr;
-char local_file[] = ./uuid_table;
-char *remote_file = NULL;
-
-if (conn-uri-user != NULL) {
-virBufferVSprintf(username, %s, conn-uri-user);
-
-if (virBufferError(username)) {
-virBufferFreeAndReset(username);
-virReportOOMError();
-goto err;
-}
-}
-
-if (virAsprintf
-(remote_file, /home/%s/libvirt_uuid_table,
- virBufferContentAndReset(username))
- 0) {
-virReportOOMError();
-goto err;
-}
-
-if (stat(local_file, local_fileinfo) == -1) {
-VIR_WARN0(Unable to stat local
[libvirt] [PATCH 2/3] PHYP: Adding basic network functions
Now adding some basic operation network functions and its UUID
helpers.
---
src/phyp/phyp_driver.c | 202 ++-
src/phyp/phyp_uuid.c | 177 ++
src/phyp/phyp_uuid.h |8 ++
3 files changed, 382 insertions(+), 5 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 6f3f49d..c44fc69 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -667,6 +667,7 @@ phypOpen(virConnectPtr conn,
size_t len = 0;
int internal_socket;
uuid_tablePtr uuid_table = NULL;
+uuid_nettablePtr uuid_nettable = NULL;
phyp_driverPtr phyp_driver = NULL;
char *char_ptr;
char *managed_system = NULL;
@@ -693,6 +694,11 @@ phypOpen(virConnectPtr conn,
goto failure;
}
+if (VIR_ALLOC(uuid_nettable) 0) {
+virReportOOMError();
+goto failure;
+}
+
if (VIR_ALLOC(connection_data) 0) {
virReportOOMError();
goto failure;
@@ -744,10 +750,15 @@ phypOpen(virConnectPtr conn,
uuid_table-nlpars = 0;
uuid_table-lpars = NULL;
+uuid_nettable-nnets = 0;
+uuid_nettable-nets = NULL;
+
if (conn-uri-path)
phyp_driver-managed_system = managed_system;
phyp_driver-uuid_table = uuid_table;
+phyp_driver-uuid_nettable = uuid_nettable;
+
if ((phyp_driver-caps = phypCapsInit()) == NULL) {
virReportOOMError();
goto failure;
@@ -759,14 +770,17 @@ phypOpen(virConnectPtr conn,
if ((phyp_driver-system_type = phypGetSystemType(conn)) == -1)
goto failure;
-if (phypUUIDTable_Init(conn) == -1)
-goto failure;
-
if (phyp_driver-system_type == HMC) {
if ((phyp_driver-vios_id = phypGetVIOSPartitionID(conn)) == -1)
goto failure;
}
+if (phypUUIDTable_Init(conn) == -1)
+goto failure;
+
+if (phypUUIDNetworkTable_Init(conn) == -1)
+goto failure;
+
return VIR_DRV_OPEN_SUCCESS;
failure:
@@ -777,6 +791,7 @@ phypOpen(virConnectPtr conn,
}
phypUUIDTable_Free(uuid_table);
+phypUUIDNetworkTable_Free(uuid_nettable);
if (session != NULL) {
libssh2_session_disconnect(session, Disconnecting...);
@@ -801,6 +816,7 @@ phypClose(virConnectPtr conn)
virCapabilitiesFree(phyp_driver-caps);
phypUUIDTable_Free(phyp_driver-uuid_table);
+phypUUIDNetworkTable_Free(phyp_driver-uuid_nettable);
VIR_FREE(phyp_driver-managed_system);
VIR_FREE(phyp_driver);
VIR_FREE(connection_data);
@@ -2813,6 +2829,182 @@ phypGetStoragePoolXMLDesc(virStoragePoolPtr pool,
unsigned int flags)
return NULL;
}
+int
+phypListNetworkMAC(virConnectPtr conn, long long *macs, int nnets)
+{
+ConnectionData *connection_data = conn-networkPrivateData;
+phyp_driverPtr phyp_driver = conn-privateData;
+LIBSSH2_SESSION *session = connection_data-session;
+int system_type = phyp_driver-system_type;
+char *managed_system = phyp_driver-managed_system;
+int vios_id = phyp_driver-vios_id;
+int exit_status = 0;
+int got = -1;
+char *cmd = NULL;
+char *ret = NULL;
+char *line, *next_line;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(buf, lshwres);
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s, managed_system);
+
+virBufferVSprintf(buf, -r virtualio --rsubtype eth --level lpar
+ |grep -v lpar_id=%d| sed -e 's/^.*mac_addr=//g',
+ vios_id);
+
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+virReportOOMError();
+return -1;
+}
+cmd = virBufferContentAndReset(buf);
+
+ret = phypExec(session, cmd, exit_status, conn);
+
+if (exit_status 0 || ret == NULL)
+goto err;
+
+/* I need to parse the textual return in order to get the macs */
+line = ret;
+got = 0;
+while (*line got nnets) {
+if (virStrToLong_ll(line, next_line, 10, macs[got]) == -1) {
+VIR_ERROR(_(Cannot parse number from '%s'), line);
+got = -1;
+goto err;
+}
+got++;
+line = next_line;
+while (*line == '\n')
+line++; /* skip \n */
+}
+
+ err:
+VIR_FREE(cmd);
+VIR_FREE(ret);
+return got;
+}
+
+int
+phypListNetworks(virConnectPtr conn, char **const names, int nnames)
+{
+ConnectionData *connection_data = conn-networkPrivateData;
+phyp_driverPtr phyp_driver = conn-privateData;
+LIBSSH2_SESSION *session = connection_data-session;
+int system_type = phyp_driver-system_type;
+char *managed_system = phyp_driver-managed_system;
+int vios_id = phyp_driver-vios_id;
+int exit_status = 0;
+int got = 0;
+int i;
+char *cmd = NULL;
+char *ret = NULL;
+char *networks = NULL;
+char *char_ptr2 = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(buf, lshwres);
+
[libvirt] [PATCH 3/3] PHYP: create, destroy and other network functions
Adding networkCreateXML, networkDestroy, networkIsActive and
networkLookupByName.
In the function phypCreateNetwork I just use the def-domain information to
create
the new network interface because of the behaviour of the HMC and the
hypervisor:
* HMC can't simply create a network interface without assigning it to a
specific
LPAR.
* I also can't assign an IP addr or any other information from the HMC or
VIOS
side, but I can control in which vlan or vswitch it will be attached - but
thought just in the simplest case scenarion now, I'll make some
improvements
in the future.
That's why I used a very simple XML for testing:
network
uuid3e3fce45-4f53-4fa7-bb32-11f34168b82b/uuid
domain name=LPAR01 /
namewhatever/name
bridge name=whatever /
/network
The only information I really need is the domain name which I'll assign the
created
network interface. Name, MAC Addr MUST be created automatically by the
hypervisor,
they're all unique. I had to put those two other tags name and bridge so
the
function virNetworkDefParseString can return successfully, otherwise it would
say
that the XML is malformed.
---
src/phyp/phyp_driver.c | 400 +++-
src/phyp/phyp_driver.h |2 +-
2 files changed, 396 insertions(+), 6 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index c44fc69..244561e 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2010 Red Hat, Inc.
- * Copyright IBM Corp. 2009
+ * Copyright IBM Corp. 2010
*
* phyp_driver.c: ssh layer to access Power Hypervisors
*
@@ -2829,6 +2829,396 @@ phypGetStoragePoolXMLDesc(virStoragePoolPtr pool,
unsigned int flags)
return NULL;
}
+static int
+networkDestroy(virNetworkPtr net)
+{
+ConnectionData *connection_data = net-conn-networkPrivateData;
+phyp_driverPtr phyp_driver = net-conn-privateData;
+LIBSSH2_SESSION *session = connection_data-session;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+uuid_nettablePtr uuid_nettable = phyp_driver-uuid_nettable;
+char *managed_system = phyp_driver-managed_system;
+int system_type = phyp_driver-system_type;
+int exit_status = 0;
+int slot_num = 0;
+char *char_ptr;
+char *cmd = NULL;
+char *ret = NULL;
+unsigned int i = 0;
+int lpar_id = 0;
+long long mac = 0;
+
+for (i = 0; i uuid_nettable-nnets; i++) {
+if (STREQ(uuid_nettable-nets[i]-name, net-name)) {
+mac = uuid_nettable-nets[i]-mac;
+break;
+}
+}
+
+/* Getting the LPAR ID */
+
+virBufferAddLit(buf, lshwres );
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s , managed_system);
+
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype slot --level slot
+ -F drc_name,lpar_id|grep %s|
+ sed -e 's/^.*,//g', net-name);
+
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+virReportOOMError();
+return -1;
+}
+cmd = virBufferContentAndReset(buf);
+
+ret = phypExec(session, cmd, exit_status, net-conn);
+
+if (exit_status 0 || ret == NULL)
+goto err;
+
+if (virStrToLong_i(ret, char_ptr, 10, lpar_id) == -1)
+goto err;
+
+/* Getting the remote slot number */
+
+virBufferAddLit(buf, lshwres );
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s , managed_system);
+
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype eth --level lpar
+ -F mac_addr,slot_num|grep %lld|
+ sed -e 's/^.*,//g', mac);
+
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+virReportOOMError();
+return -1;
+}
+cmd = virBufferContentAndReset(buf);
+
+ret = phypExec(session, cmd, exit_status, net-conn);
+
+if (exit_status 0 || ret == NULL)
+goto err;
+
+if (virStrToLong_i(ret, char_ptr, 10, slot_num) == -1)
+goto err;
+
+/* excluding interface */
+
+virBufferAddLit(buf, chhwres );
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s , managed_system);
+
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype eth
+ --id %d -o r -s %d, lpar_id, slot_num);
+
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+virReportOOMError();
+return -1;
+}
+cmd = virBufferContentAndReset(buf);
+
+ret = phypExec(session, cmd, exit_status, net-conn);
+
+if (exit_status 0 || ret != NULL)
+goto err;
+
+if (phypUUIDTable_RemNetwork(net-conn, mac) 0)
+goto err;
+
+VIR_FREE(cmd);
+VIR_FREE(ret);
+return 0;
+
+ err:
+VIR_FREE(cmd);
+VIR_FREE(ret);
+return -1;
+}
+
+static virNetworkPtr
+phypCreateNetwork(virConnectPtr conn, const char *xml)
[libvirt] [PATCH 1/4] conf: Convert ParseString to use STRPREFIX
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/util/conf.c | 19 +++
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index ba1a384..a31bbc4 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -394,17 +394,20 @@ virConfParseString(virConfParserCtxtPtr ctxt)
return NULL;
}
NEXT;
-} else if ((ctxt-cur + 6 ctxt-end) (ctxt-cur[0] == '')
- (ctxt-cur[1] == '') (ctxt-cur[2] == '')) {
+} else if ((ctxt-cur + 6 ctxt-end)
+ (STRPREFIX(ctxt-cur, \\\))) {
+/* String starts with python-style triple quotes */
ctxt-cur += 3;
base = ctxt-cur;
-while ((ctxt-cur + 2 ctxt-end) (ctxt-cur[0] == '')
- (ctxt-cur[1] == '') (ctxt-cur[2] == '')) {
- if (CUR == '\n') ctxt-line++;
- NEXT;
+
+while ((ctxt-cur + 2 ctxt-end)
+ (STRPREFIX(ctxt-cur, \\\))) {
+if (CUR == '\n')
+ctxt-line++;
+NEXT;
}
-if ((ctxt-cur[0] != '') || (ctxt-cur[1] != '') ||
-(ctxt-cur[2] != '')) {
+
+if (!STRPREFIX(ctxt-cur, \\\)) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _(unterminated string));
return(NULL);
}
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] xen: parsing and sexpr escaping fixes
This series fixes some xen XM parsing and sexpr generation issues. The first two patches fix parsing /etc/xen files that use python style triple quotes (which libvirt will actually generate in certain situations). The last two patches fix generating xend sexpr with the reserved characters \ or ' Cole Robinson (4): conf: Convert ParseString to use STRPREFIX conf: Fix parsing python style triple quotes xend: urlencode: Properly escape '' xend: Escape reserved sexpr characters docs/schemas/domain.rng| 10 +- src/util/buf.c | 79 +++ src/util/buf.h |1 + src/util/conf.c| 20 +++-- src/xen/xend_internal.c| 116 +++ tests/xmconfigdata/test-escape-paths.cfg |2 +- tests/xmconfigdata/test-escape-paths.xml |5 + tests/xml2sexprdata/xml2sexpr-escape.sexpr |1 + tests/xml2sexprdata/xml2sexpr-escape.xml | 24 ++ tests/xml2sexprtest.c |1 + 10 files changed, 193 insertions(+), 66 deletions(-) create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.sexpr create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.xml -- 1.7.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Allow probing of image formats without version information
Disk image formats that wish to opt-out of version validation are supposed to set versionOffset to -1 in their fileTypeInfo entry. By unconditionally returning False for these formats, virStorageFileMatchesVersion() incorrectly reports a version mismatch when the test was actually skipped. The correct behavior is to return True so these formats can be successfully probed using the magic bytes alone. Signed-off-by: Adam Litke a...@us.ibm.com --- src/util/storage_file.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 4098383..f8ab168 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -478,7 +478,7 @@ virStorageFileMatchesVersion(int format, /* Validate version number info */ if (fileTypeInfo[format].versionOffset == -1) -return false; +return true; if ((fileTypeInfo[format].versionOffset + 4) buflen) return false; -- 1.7.3.2.164.g6f10c -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] xend: urlencode: Properly escape ''
Since we send the sexpr to xend via HTTP, we need to properly escape
''
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/xen/xend_internal.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index d6d66bd..3ccadde 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -710,6 +710,7 @@ urlencode(const char *string)
switch (string[i]) {
case ' ':
case '\n':
+case '':
snprintf(ptr, 4, %%%02x, string[i]);
ptr += 3;
break;
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes
An incorrect check broke matching the closing set of quotes. Update
tests to cover this case for XM config files, and update the domain schema
to allow more path characters.
Signed-off-by: Cole Robinson crobi...@redhat.com
---
docs/schemas/domain.rng | 10 +-
src/util/conf.c |3 ++-
tests/xmconfigdata/test-escape-paths.cfg |2 +-
tests/xmconfigdata/test-escape-paths.xml |5 +
4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index bbbc846..870bea1 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -2028,27 +2028,27 @@
/define
define name=filePath
data type=string
- param name=pattern[a-zA-Z0-9_\.\+\-amp;/%]+/param
+ param
name=pattern[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]+/param
/data
/define
define name=absFilePath
data type=string
- param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]+/param
+ param
name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]+/param
/data
/define
define name=absDirPath
data type=string
- param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
+ param
name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param
/data
/define
define name=devicePath
data type=string
- param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
+ param
name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param
/data
/define
define name=deviceName
data type=string
- param name=pattern[a-zA-Z0-9_\.\-:/]+/param
+ param
name=pattern[a-zA-Z0-9_\.\-\\amp;quot;apos;lt;gt;:/]+/param
/data
/define
define name=bridgeMode
diff --git a/src/util/conf.c b/src/util/conf.c
index a31bbc4..d9a7603 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt)
ctxt-cur += 3;
base = ctxt-cur;
+/* Find the ending triple quotes */
while ((ctxt-cur + 2 ctxt-end)
- (STRPREFIX(ctxt-cur, \\\))) {
+ !(STRPREFIX(ctxt-cur, \\\))) {
if (CUR == '\n')
ctxt-line++;
NEXT;
diff --git a/tests/xmconfigdata/test-escape-paths.cfg
b/tests/xmconfigdata/test-escape-paths.cfg
index f9f2cb8..e3e6db9 100644
--- a/tests/xmconfigdata/test-escape-paths.cfg
+++ b/tests/xmconfigdata/test-escape-paths.cfg
@@ -19,7 +19,7 @@ vnc = 1
vncunused = 1
vnclisten = 127.0.0.1
vncpasswd = 123poi
-disk = [ phy:/dev/HostVG/XenGuest2,hda,w,
file:/root/boot.isotest,hdc:cdrom,r ]
+disk = [ phy:/dev/HostVG/XenGuest2,hda,w,
file:/root/boot.isotest,hdc:cdrom,r, phy:/dev/HostVG/XenGuest',hdb,w ]
vif = [
mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu ]
parallel = none
serial = none
diff --git a/tests/xmconfigdata/test-escape-paths.xml
b/tests/xmconfigdata/test-escape-paths.xml
index dabf492..13e6e29 100644
--- a/tests/xmconfigdata/test-escape-paths.xml
+++ b/tests/xmconfigdata/test-escape-paths.xml
@@ -31,6 +31,11 @@
target dev='hdc' bus='ide'/
readonly/
/disk
+disk type='block' device='disk'
+ driver name='phy'/
+ source dev='/dev/HostVG/XenGuestapos;quot;'/
+ target dev='hdb' bus='ide'/
+/disk
interface type='bridge'
mac address='00:16:3e:66:92:9c'/
source bridge='xenbr1'/
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] QED: Basic support for QED images
Add an entry in fileTypeInfo for QED image files.
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefan.hajno...@uk.ibm.com
Cc: Anthony Liguori aligu...@linux.vnet.ibm.com
---
src/util/storage_file.c |9 -
src/util/storage_file.h |1 +
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index f8ab168..27aad26 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -43,7 +43,7 @@ VIR_ENUM_IMPL(virStorageFileFormat,
VIR_STORAGE_FILE_LAST,
raw, dir, bochs,
cloop, cow, dmg, iso,
- qcow, qcow2, vmdk, vpc)
+ qcow, qcow2, qed, vmdk, vpc)
enum lv_endian {
LV_LITTLE_ENDIAN = 1, /* 1234 */
@@ -104,6 +104,8 @@ static int vmdk4GetBackingStore(char **, int *,
#define QCOW2_HDR_EXTENSION_END 0
#define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
+#define QED_HDR_IMAGE_SIZE 40
+
/* VMDK needs at least this to find backing store,
* other formats need less */
#define STORAGE_MAX_HEAD (20*512)
@@ -151,6 +153,11 @@ static struct FileTypeInfo const fileTypeInfo[] = {
LV_BIG_ENDIAN, 4, 2,
QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW2_HDR_CRYPT, qcow2GetBackingStore,
},
+[VIR_STORAGE_FILE_QED] = {
+QED\0, NULL,
+LV_LITTLE_ENDIAN, -1, -1,
+QED_HDR_IMAGE_SIZE, 8, 1, -1, NULL,
+},
[VIR_STORAGE_FILE_VMDK] = {
KDMV, NULL,
LV_LITTLE_ENDIAN, 4, 1,
diff --git a/src/util/storage_file.h b/src/util/storage_file.h
index a3703f5..c4d4650 100644
--- a/src/util/storage_file.h
+++ b/src/util/storage_file.h
@@ -38,6 +38,7 @@ enum virStorageFileFormat {
VIR_STORAGE_FILE_ISO,
VIR_STORAGE_FILE_QCOW,
VIR_STORAGE_FILE_QCOW2,
+VIR_STORAGE_FILE_QED,
VIR_STORAGE_FILE_VMDK,
VIR_STORAGE_FILE_VPC,
VIR_STORAGE_FILE_LAST,
--
1.7.3.2.164.g6f10c
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] xend: Escape reserved sexpr characters
If we don't escape ' or \ xend can't parse the generated sexpr. This
might over apply the EscapeSexpr routine, but it shouldn't hurt.
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/util/buf.c | 79 +++
src/util/buf.h |1 +
src/xen/xend_internal.c| 115 +++-
tests/xml2sexprdata/xml2sexpr-escape.sexpr |1 +
tests/xml2sexprdata/xml2sexpr-escape.xml | 24 ++
tests/xml2sexprtest.c |1 +
6 files changed, 169 insertions(+), 52 deletions(-)
create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.sexpr
create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.xml
diff --git a/src/util/buf.c b/src/util/buf.c
index 553e2a0..90034ad 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -379,6 +379,85 @@ err:
}
/**
+ * virBufferEscapeSexpr:
+ * @buf: the buffer to dump
+ * @format: a printf like format string but with only one %s parameter
+ * @str: the string argument which need to be escaped
+ *
+ * Do a formatted print with a single string to an sexpr buffer. The string
+ * is escaped to avoid generating a sexpr that xen will choke on. This
+ * doesn't fully escape the sexpr, just enough for our code to work.
+ */
+void
+virBufferEscapeSexpr(const virBufferPtr buf,
+ const char *format,
+ const char *str)
+{
+int size, count, len, grow_size;
+char *escaped, *out;
+const char *cur;
+
+if ((format == NULL) || (buf == NULL) || (str == NULL))
+return;
+
+if (buf-error)
+return;
+
+len = strlen(str);
+if (VIR_ALLOC_N(escaped, 2 * len + 1) 0) {
+virBufferNoMemory(buf);
+return;
+}
+
+cur = str;
+out = escaped;
+while (*cur != 0) {
+switch (*cur) {
+case '\\':
+case '\'':
+*out++ = '\\';
+/* fallthrough */
+default:
+*out++ = *cur;
+}
+cur++;
+}
+*out = 0;
+
+if ((buf-use = buf-size)
+virBufferGrow(buf, 100) 0) {
+goto err;
+}
+
+size = buf-size - buf-use;
+if ((count = snprintf(buf-content[buf-use], size,
+ format, (char *)escaped)) 0) {
+buf-error = 1;
+goto err;
+}
+
+/* Grow buffer if necessary and retry */
+if (count = size) {
+buf-content[buf-use] = 0;
+grow_size = (count + 1 1000) ? count + 1 : 1000;
+if (virBufferGrow(buf, grow_size) 0) {
+goto err;
+}
+size = buf-size - buf-use;
+
+if ((count = snprintf(buf-content[buf-use], size,
+ format, (char *)escaped)) 0) {
+buf-error = 1;
+goto err;
+}
+}
+buf-use += count;
+
+err:
+VIR_FREE(escaped);
+}
+
+/**
* virBufferURIEncodeString:
* @buf: the buffer to append to
* @str: the string argument which will be URI-encoded
diff --git a/src/util/buf.h b/src/util/buf.h
index 6616898..54f4de5 100644
--- a/src/util/buf.h
+++ b/src/util/buf.h
@@ -45,6 +45,7 @@ void virBufferVSprintf(const virBufferPtr buf, const char
*format, ...)
void virBufferStrcat(const virBufferPtr buf, ...)
ATTRIBUTE_SENTINEL;
void virBufferEscapeString(const virBufferPtr buf, const char *format, const
char *str);
+void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, const
char *str);
void virBufferURIEncodeString (const virBufferPtr buf, const char *str);
# define virBufferAddLit(buf_, literal_string_) \
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 3ccadde..7a36fb0 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -522,6 +522,7 @@ xend_op_ext(virConnectPtr xend, const char *path, const
char *key, va_list ap)
}
content = virBufferContentAndReset(buf);
+DEBUG(xend op: %s\n, content);
ret = http2unix(xend_post(xend, path, content));
VIR_FREE(content);
@@ -4605,8 +4606,6 @@ virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn,
const char *xmlDesc) {
goto error;
}
-DEBUG(Defining w/ sexpr: \n%s, sexpr);
-
ret = xend_op(conn, , op, new, config, sexpr, NULL);
VIR_FREE(sexpr);
if (ret != 0) {
@@ -5297,11 +5296,12 @@ xenDaemonFormatSxprChr(virDomainChrDefPtr def,
case VIR_DOMAIN_CHR_TYPE_FILE:
case VIR_DOMAIN_CHR_TYPE_PIPE:
-virBufferVSprintf(buf, %s:%s, type, def-data.file.path);
+virBufferVSprintf(buf, %s:, type);
+virBufferEscapeSexpr(buf, %s, def-data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
-virBufferVSprintf(buf, %s, def-data.file.path);
+virBufferEscapeSexpr(buf, %s, def-data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_TCP:
@@ -5322,8 +5322,9 @@ xenDaemonFormatSxprChr(virDomainChrDefPtr def,
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
-
[libvirt] [PATCH 0/4] Support the QED disk image format (V2)
Changes since V1: - Fix virStorageFileMatchesVersion() for formats without version info - Allow backingStore format probing for QED images since it is safe Qemu is about to gain support for a new disk image format: QED. Details on this format (including specification) can be found here: http://wiki.qemu.org/Features/QED. This short series of patches allows QED images to be used with libvirt. Adam Litke (4): Allow probing of image formats without version information QED: Basic support for QED images storage_file: Add a new flag to mark backing files that are safe to probe Support for probing qed image metadata src/conf/domain_conf.c |4 ++ src/util/storage_file.c | 89 +-- src/util/storage_file.h |2 + 3 files changed, 92 insertions(+), 3 deletions(-) -- 1.7.3.2.164.g6f10c -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Support for probing qed image metadata
Implement getBackingStore() for QED images. The header format is defined in
the QED spec: http://wiki.qemu.org/Features/QED .
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefan.hajno...@uk.ibm.com
Cc: Anthony Liguori aligu...@linux.vnet.ibm.com
---
src/util/storage_file.c | 78 ++-
1 files changed, 77 insertions(+), 1 deletions(-)
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index b656557..b195ef7 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -89,6 +89,8 @@ static int qcow2GetBackingStore(char **, int *,
const unsigned char *, size_t);
static int vmdk4GetBackingStore(char **, int *,
const unsigned char *, size_t);
+static int
+qedGetBackingStore(char **, int *, const unsigned char *, size_t);
#define QCOWX_HDR_VERSION (4)
#define QCOWX_HDR_BACKING_FILE_OFFSET (QCOWX_HDR_VERSION+4)
@@ -105,6 +107,11 @@ static int vmdk4GetBackingStore(char **, int *,
#define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
#define QED_HDR_IMAGE_SIZE 40
+#define QED_HDR_FEATURES_OFFSET 16
+#define QED_HDR_BACKING_FILE_OFFSET 56
+#define QED_HDR_BACKING_FILE_SIZE 60
+#define QED_F_BACKING_FILE 0x01
+#define QED_F_BACKING_FORMAT_NO_PROBE 0x04
/* VMDK needs at least this to find backing store,
* other formats need less */
@@ -156,7 +163,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
[VIR_STORAGE_FILE_QED] = {
QED\0, NULL,
LV_LITTLE_ENDIAN, -1, -1,
-QED_HDR_IMAGE_SIZE, 8, 1, -1, NULL,
+QED_HDR_IMAGE_SIZE, 8, 1, -1, qedGetBackingStore,
},
[VIR_STORAGE_FILE_VMDK] = {
KDMV, NULL,
@@ -416,6 +423,75 @@ cleanup:
return ret;
}
+static unsigned long
+qedGetHeaderUL(const unsigned char *loc)
+{
+return ( ((unsigned long)loc[3] 24)
+ | ((unsigned long)loc[2] 16)
+ | ((unsigned long)loc[1] 8)
+ | ((unsigned long)loc[0] 0));
+}
+
+static unsigned long long
+qedGetHeaderULL(const unsigned char *loc)
+{
+return ( ((unsigned long)loc[7] 56)
+ | ((unsigned long)loc[6] 48)
+ | ((unsigned long)loc[5] 40)
+ | ((unsigned long)loc[4] 32)
+ | ((unsigned long)loc[3] 24)
+ | ((unsigned long)loc[2] 16)
+ | ((unsigned long)loc[1] 8)
+ | ((unsigned long)loc[0] 0));
+}
+
+static int
+qedGetBackingStore(char **res,
+ int *format,
+ const unsigned char *buf,
+ size_t buf_size)
+{
+unsigned long long flags;
+unsigned long offset, size;
+
+*res = NULL;
+/* Check if this image has a backing file */
+if (buf_size QED_HDR_FEATURES_OFFSET+8)
+return BACKING_STORE_INVALID;
+flags = qedGetHeaderULL(buf + QED_HDR_FEATURES_OFFSET);
+if (!(flags QED_F_BACKING_FILE))
+return BACKING_STORE_OK;
+
+/* Parse the backing file */
+if (buf_size QED_HDR_BACKING_FILE_OFFSET+8)
+return BACKING_STORE_INVALID;
+offset = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_OFFSET);
+if (offset buf_size)
+return BACKING_STORE_INVALID;
+size = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_SIZE);
+if (size == 0)
+return BACKING_STORE_OK;
+if (offset + size buf_size || offset + size offset)
+return BACKING_STORE_INVALID;
+if (size + 1 == 0)
+return BACKING_STORE_INVALID;
+if (VIR_ALLOC_N(*res, size + 1) 0) {
+virReportOOMError();
+return BACKING_STORE_ERROR;
+}
+memcpy(*res, buf + offset, size);
+(*res)[size] = '\0';
+
+if (format) {
+if (flags QED_F_BACKING_FORMAT_NO_PROBE)
+*format = virStorageFileFormatTypeFromString(raw);
+else
+*format = VIR_STORAGE_FILE_AUTO_SAFE;
+}
+
+return BACKING_STORE_OK;
+}
+
/**
* Return an absolute path corresponding to PATH, which is absolute or relative
* to the directory containing BASE_FILE, or NULL on error
--
1.7.3.2.164.g6f10c
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] storage_file: Add a new flag to mark backing files that are safe to probe
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/conf/domain_conf.c |4
src/util/storage_file.c |2 +-
src/util/storage_file.h |1 +
3 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d11785..a08c846 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7825,6 +7825,10 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
if (format == VIR_STORAGE_FILE_AUTO
!allowProbing)
format = VIR_STORAGE_FILE_RAW; /* Stops further recursion */
+
+/* Allow probing for image formats that are safe */
+if (format == VIR_STORAGE_FILE_AUTO_SAFE)
+format = VIR_STORAGE_FILE_AUTO;
} while (nextpath);
ret = 0;
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 27aad26..b656557 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -41,7 +41,7 @@
VIR_ENUM_IMPL(virStorageFileFormat,
VIR_STORAGE_FILE_LAST,
- raw, dir, bochs,
+ raw, probe, dir, bochs,
cloop, cow, dmg, iso,
qcow, qcow2, qed, vmdk, vpc)
diff --git a/src/util/storage_file.h b/src/util/storage_file.h
index c4d4650..13c731f 100644
--- a/src/util/storage_file.h
+++ b/src/util/storage_file.h
@@ -30,6 +30,7 @@
enum virStorageFileFormat {
VIR_STORAGE_FILE_AUTO = -1,
VIR_STORAGE_FILE_RAW = 0,
+VIR_STORAGE_FILE_AUTO_SAFE,
VIR_STORAGE_FILE_DIR,
VIR_STORAGE_FILE_BOCHS,
VIR_STORAGE_FILE_CLOOP,
--
1.7.3.2.164.g6f10c
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/10] memory: make it safer to expand arrays
On 11/19/2010 12:33 AM, Wen Congyang wrote:
At 2010-11-18 12:28, Eric Blake Write:
* src/util/memory.h (VIR_REALLOC_N): Update docs.
(VIR_EXPAND_N, VIR_SHRINK_N): New macros.
(virAlloc, virAllocN, virReallocN, virAllocVar, virFree): Add some
gcc attributes.
There may be a bug in this patch.
Well, it would be patch 3 that touched the file where your backtrace
points, so it would be the overall patch series and not this patch
(patch 1) to blame.
Test the libvirtd without --dameon, I find that:
[r...@localhost newest]# libvirtd
Segmentation fault (core dumped)
[r...@localhost newest]#
The folling is the output of the command 'gdb libvirtd core':
[r...@localhost newest]# gdb /usr/sbin/libvirtd core.8996
snip
Core was generated by `libvirtd'.
Program terminated with signal 11, Segmentation fault.
#0 0x0041a181 in qemudDispatchServer (server=0x209dcd0, sock=value
optimized out) at libvirtd.c:1459
1459 server-clients[server-nclients++] = client;
snip
(gdb) bt
#0 0x0041a181 in qemudDispatchServer (server=0x209dcd0, sock=value
optimized out) at libvirtd.c:1459
#1 0x0041a6f1 in qemudDispatchServerEvent (watch=5, fd=8, events=1,
opaque=0x209dcd0) at libvirtd.c:2225
#2 0x00415b71 in virEventDispatchHandles () at event.c:467
#3 virEventRunOnce () at event.c:592
#4 0x004180e9 in qemudOneLoop () at libvirtd.c:2234
#5 0x004183db in qemudRunLoop (opaque=0x209dcd0) at libvirtd.c:2343
#6 0x003ffec077e1 in start_thread () from /lib64/libpthread.so.0
#7 0x003ffe4e153d in clone () from /lib64/libc.so.6
(gdb) p server-clients
$2 = (struct qemud_client **) 0x0
I'm having problems reproducing this, and don't see any obvious
explanations for this in the code. qemuDispatchServer has:
if (server-nclients = max_clients) {
VIR_ERROR(_(Too many active clients (%d), dropping connection
from %s),
max_clients, addrstr);
goto error;
}
if (VIR_RESIZE_N(server-clients, server-nclients_max,
server-nclients, 1) 0) {
VIR_ERROR0(_(Out of memory allocating clients));
goto error;
}
...
server-clients[server-nclients++] = client;
so the only way to get to the crashing line is to get through a
successful VIR_RESIZE_N, but VIR_RESIZE_N is not successful unless it
updates server-clients to be non-NULL.
Can you do any further debugging that might explain why it is failing
for you, and something I might have missed?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 01/11] Fix parsing of port attribute in storage XML configuration
On 11/12/2010 11:36 AM, Daniel P. Berrange wrote: On Fri, Nov 12, 2010 at 10:21:16AM -0700, Eric Blake wrote: On 11/12/2010 09:22 AM, Daniel P. Berrange wrote: The XML docs describe a 'port' attribute for the storage source host element, but the parser never handled it. * docs/schemas/storagepool.rng: Define port attribute * src/conf/storage_conf.c: Add missing parsing/formatting of host port number * src/conf/storage_conf.h: Remove bogus/unused 'protocol' field --- docs/schemas/storagepool.rng |5 + Missing corresponding docs/formatstorage.html.in change. As per the commit message, this is already documented, but not implemented ! Good to know. diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 54eb802..8f067f3 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -186,6 +186,11 @@ attribute name='name' text/ /attribute + optional +attribute name='port' + text/ Is text really appropriate? I guess it should reference a port number I don't see this patch applied yet. ACK with that nit to the schema fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] allow '#' as valid character for domain name
On 11/19/2010 05:29 AM, Osier Yang wrote: * docs/schemas/domain.rng --- docs/schemas/domain.rng |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index bbbc846..815134d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -2003,7 +2003,7 @@ /define define name=domainName data type=string -param name=pattern[A-Za-z0-9_\.\+\-amp;:/]+/param +param name=pattern[A-Za-z0-9_\.\+\-\\#amp;:/]+/param /data /define define name=diskSerial What's your motivation for this? If domain.rng is used similarly to the other .rng files I'm more familiar with, it's only actually examined during the tests run as part of make check, so it won't have any effect on actual operation. Is this what you intended? # seems like a problematic character to put in a domain name - for example it would need to be escaped or quoted if it was ever on a commandline - what happens when that name gets passed to qemu, for example? Or a user-written shell script that calls virsh? Also, virt-manager doesn't allow it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/11] Stop iSCSI targets automatically logging back in after logout
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
The Linux iSCSI initiator toolchain has the dubious feature that
if you ever run the 'sendtargets' command to merely query what
targets are available from a server, the results will be recorded
in /var/lib/iscsi. Any time the '/etc/init.d/iscsi' script runs
in the future, it will then automatically login to all those
targets. /etc/init.d/iscsi is automatically run whenever a NIC
comes online.
Is that worth reporting as a bug in the iscsi toolchain? At any rate,
we need this patch whether or not that tool changes behavior to
something more sane. However, I'm not sure this is ready for ack
without answers to some questions first:
To stop this stupid behaviour, we need to run
iscsiadm --portal $PORTAL --target $TARGET
--op update --name node.startup --value manual
+static int
+virStorageBackendISCSIGetTargets(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ char **const groups,
+ void *data)
+{
+struct virStorageBackendISCSITargetList *list = data;
+char *target;
+
+if (!(target = strdup(groups[1]))) {
+virReportOOMError();
+return -1;
+}
+
+if (VIR_REALLOC_N(list-targets, list-ntargets + 1) 0) {
Up to you if you want to rebase this to use VIR_RESIZE_N (or
VIR_EXPAND_N), or to just leave that to me as a subsequent followup
patch (I'm already searching through the code base for other instances
to convert; one more won't hurt me).
static int
-virStorageBackendISCSIScanTargets(const char *portal)
+virStorageBackendISCSIScanTargets(const char *portal,
+ const char *initiatoriqn,
+ size_t *ntargetsret,
+ char ***targetsret)
{
-const char *const sendtargets[] = {
-ISCSIADM, --mode, discovery, --type, sendtargets,
--portal, portal, NULL
+/**
+ *
+ * The output of sendtargets is very simple, just two columns,
+ * portal then target name
+ *
+ * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo0.bf6d84
+ * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo1.bf6d84
+ * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo2.bf6d84
+ * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo3.bf6d84
+ */
+const char *regexes[] = {
+^\\s*(\\S+)\\s+(\\S+)\\s*$
\s and \S are GNU-isms, and regcomp() on other platforms will reject it;
is this regex only used on Linux, or do we need to be portable to iscsi
implementations on other platforms?
+};
+int vars[] = { 2 };
+const char *const cmdsendtarget[] = {
+ISCSIADM, --mode, discovery, --type, sendtargets,
+--portal, portal, NULL
};
-if (virRun(sendtargets, NULL) 0) {
+struct virStorageBackendISCSITargetList list;
+int i;
+int exitstatus;
+
+memset(list, 0, sizeof(list));
+
+if (virStorageBackendRunProgRegex(NULL, /* No pool for callback */
+ cmdsendtarget,
+ 1,
+ regexes,
+ vars,
+ virStorageBackendISCSIGetTargets,
+ list,
+ exitstatus) 0) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR,
- _(Failed to run %s to get target list),
- sendtargets[0]);
+ %s, _(lvs command failed));
Should this message be about iscsiadm rather than lvs?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 04/11] Add support for iSCSI target auto-discovery
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
Since the previous patch added support for parsing the output of
the 'sendtargets' command, it is now trivial to support the
storage pool discovery API.
Given a hostname and optional portnumber and initiator IQN,
the code can return a full list of storage pool source docs,
each one representing a iSCSI target.
* src/storage/storage_backend_iscsi.c: Wire up target
auto-discovery
---
+static char *
+virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
+ const char *srcSpec,
+ unsigned int flags ATTRIBUTE_UNUSED)
+{
Should this check that flags==0?
ACK with that nit addressed.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 05/11] Switch the virsh XML generation to use virBuffer instead of virAsprintf
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote: The code generating XML for storage pool source discovery is hardcoded to only allow a hostname and optional port number. Refactor this code to make it easier to add support for extra parameters. * tools/virsh.c: Refactor XML generator --- tools/virsh.c | 26 ++ 1 files changed, 10 insertions(+), 16 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d15a8df..6fc1b47 100644 --- a/tools/virsh.c +virBuffer buf = VIR_BUFFER_INITIALIZER; +virBufferAddLit(buf, source\n); +virBufferVSprintf(buf, host name='%.*s',(int)hostlen, host); +if (port) +virBufferVSprintf(buf, port='%s', port); +virBufferAddLit(buf, /\n); +virBufferAddLit(buf, /source\n); Should these two lines be combined into a single virBufferAddLit? Then again, if later patches are adding in new elements besides host, it makes sense to insert between these two lines. ACK either way. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 06/11] Allow iSCSI IQN to be set with find-storage-pool-sources-as command
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
Allow an iSCSI initiator IQN to be set with the XML for the
find-storage-pool-sources-as virsh command
* tools/virsh.c: Add iSCSI IQN support
---
tools/virsh.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
@@ -5782,6 +5787,11 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const
vshCmd * cmd ATTRIBUTE_UNUSED)
if (port)
virBufferVSprintf(buf, port='%s', port);
virBufferAddLit(buf, /\n);
+if (initiator) {
+virBufferAddLit(buf, initiator\n);
+virBufferVSprintf(buf, iqn name='%s'/\n, initiator);
+virBufferAddLit(buf, /initiator\n);
+}
Yep, answering my own question on patch 5/11, you stuck something in the
middle.
ACK.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 07/11] Remove bogus port handling code in virsh
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
The find-storage-pool-sources-as command takes two arguments,
a hostname and a port number. For some reason the code would
also then look for a port number appended to the hostname
string by searching for ':'. This totally breaks if the user
gives an IPv6 address, and is redundant, since you can already
provide a port as a separate argument
* tools/virsh.c: Remove bogus port number handling code
---
tools/virsh.c | 11 ++-
1 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 69a42e8..a840758 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -5772,15 +5772,8 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const
vshCmd * cmd ATTRIBUTE_UNUSED)
if (host) {
size_t hostlen = strlen(host);
char *port = vshCommandOptString(cmd, port, found);
-if (!found) {
-port = strrchr(host, ':');
-if (port) {
-if (*(++port))
-hostlen = port - host - 1;
-else
-port = NULL;
-}
-}
+if (!found)
+port = NULL;
Slight change which may break existing scripts that used an undocumented
method, but makes sense given that the documentation calls out both
arguments, and in light of IPv6 hostnames.
virBuffer buf = VIR_BUFFER_INITIALIZER;
virBufferAddLit(buf, source\n);
virBufferVSprintf(buf, host name='%.*s',(int)hostlen, host);
However, given that we are no longer computing hostlen as anything other
than strlen(host), I would recommend that you respin this patch to
completely get rid of hostlen, and use %s instead of %.*s.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 08/11] Check whether pools are already active upon libvirtd startup
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
When libvirt starts up all storage pools default to the inactive
state, even if the underlying storage is already active on the
host. This introduces a new API into the internal storage backend
drivers that checks whether a storage pool is already active. If
the pool is active at libvirtd startup, the volume list will be
immediately populated.
+++ b/src/storage/storage_backend_mpath.c
@@ -27,6 +27,8 @@
#include stdio.h
#include dirent.h
#include fcntl.h
+#include sys/stat.h
+#include sys/types.h
sys/types.h is generally redundant, now that POSIX 2008 requires most
headers to be self-contained.
+static int
+virStorageBackendMpathCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+bool *isActive)
+{
+const char *path = /dev/mpath;
+
+*isActive = false;
+
+struct stat sb;
+if (stat(path, sb) == 0)
+*isActive = true;
General observation: using stat() for existence checks is generally
slower than access(,F_OK), because the kernel has to do more work to
populate the result buffer that you then ignore. While what you have
works, maybe you should consider rewriting this to use access().
+++ b/src/storage/storage_backend_scsi.c
@@ -27,6 +27,9 @@
#include stdio.h
#include dirent.h
#include fcntl.h
+#include sys/types.h
+#include sys/stat.h
+#include sys/wait.h
Why sys/wait.h?
+++ b/src/storage/storage_driver.c
@@ -68,17 +68,29 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {
for (i = 0 ; i driver-pools.count ; i++) {
virStoragePoolObjPtr pool = driver-pools.objs[i];
+virStorageBackendPtr backend;
+bool started = false;
virStoragePoolObjLock(pool);
-if (pool-autostart
-!virStoragePoolObjIsActive(pool)) {
-virStorageBackendPtr backend;
-if ((backend = virStorageBackendForType(pool-def-type)) ==
NULL) {
-VIR_ERROR(_(Missing backend %d), pool-def-type);
-virStoragePoolObjUnlock(pool);
-continue;
-}
+if ((backend = virStorageBackendForType(pool-def-type)) == NULL) {
+VIR_ERROR(_(Missing backend %d), pool-def-type);
+virStoragePoolObjUnlock(pool);
+continue;
+}
+if (backend-checkPool
+backend-checkPool(NULL, pool, started) 0) {
+virErrorPtr err = virGetLastError();
+VIR_ERROR(_(Failed to initialize storage pool '%s': %s),
+ pool-def-name, err ? err-message :
+ no error message found);
Missing _() around that last string.
ACK to the overall idea, but you may want to respin this given my comments.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] allow '#' as valid character for domain name
On 11/19/2010 11:00 AM, Laine Stump wrote: -param name=pattern[A-Za-z0-9_\.\+\-amp;:/]+/param +param name=pattern[A-Za-z0-9_\.\+\-\\#amp;:/]+/param /data /define define name=diskSerial What's your motivation for this? If domain.rng is used similarly to the other .rng files I'm more familiar with, it's only actually examined during the tests run as part of make check, so it won't have any effect on actual operation. Is this what you intended? Additionally, it would be a good idea to add a .xml file somewhere in the tests/ hierarchy that has such a domain name, as a way of getting test exposure of both the RelaxNG schema validation and of actual XML handling in libvirt itself. # seems like a problematic character to put in a domain name - for example it would need to be escaped or quoted if it was ever on a commandline Only if it is the first character of the domain name, but yes, this aspect alone makes me need to see a concrete example of someone using a domain like this before we can add support for it. $ echo #ab $ echo a#b a#b -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 09/11] Fix error codes returned when a storage pool is inactive
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote: Many operations are not valid on inactive storage pools. The storage driver is currently returning VIR_ERR_INTERNAL_ERROR in these cases, rather than the more suitable error code VIR_ERR_OPERATION_INVALID * src/storage/storage_driver.c: Fix error code when pool is not active ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 10/11] Improve SCSI volume key and name generation
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
The SCSI volumes currently get a name like '17:0:0:1' based
on $host:$bus:$target:$lun. The names are intended to be unique
per pool and stable across pool restarts. The inclusion of the
$host component breaks this, because the $host number for iSCSI
pools is dynamically allocated by the kernel at time of login.
This changes the name to be 'unit:0:0:1', ie removes the leading
host component. THe 'unit:' prefix is just to ensure the volume
s/THe/The/
name doesn't start with a number and make it clearer when seen
out of context.
The SCSI volumes also get a 'key' field based on the fully
qualified volume path. All SCSI volumes have a unique serial
available in hardware which can be obtained by sending a
suitable SCSI command. Call out to udev's 'scsi_id' command
to fetch this value
I don't know if you adequately answered the usage questions raised by
others, but from the code review aspect, I hadn't seen an ack yet.
* src/storage/storage_backend_scsi.c: Improve key and name
field value stability and uniqness
s/uniqness/uniqueness/
+static char *
+virStorageBackendSCSISerial(const char *dev)
+{
+const char *cmdargv[] = {
+/lib/udev/scsi_id,
+--replace-whitespace,
+--whitelisted,
+--device, dev,
+NULL
+};
+int fd = -1;
+pid_t child;
+FILE *list = NULL;
+char line[1024];
+char *serial = NULL;
+int err;
+int exitstatus;
+
+/* Run the program and capture its output */
+if (virExec(cmdargv, NULL, NULL,
+child, -1, fd, NULL, VIR_EXEC_NONE) 0)
+goto cleanup;
All the more reason for me to get my virCommand patch cleaned up per
comments and pushed. This patch seems better off to rebase on top of
virCommand.
+
+if ((list = fdopen(fd, r)) == NULL) {
VIR_FDOPEN (hmm, I really need to get that promised syntax-check going
for fdopen/[f]close).
static int
virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
-uint32_t host,
+uint32_t host ATTRIBUTE_UNUSED,
uint32_t bus,
uint32_t target,
uint32_t lun,
@@ -183,7 +240,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
vol-type = VIR_STORAGE_VOL_BLOCK;
-if (virAsprintf((vol-name), %u.%u.%u.%u, host, bus, target, lun)
0) {
+/* 'host' is dynamically allocated by the kernel, first come,
+ * first served, per HBA. As such it isn't suitable for use
+ * in the volume name. We only need uniquness per-pool, so
s/uniquness/uniqueness/ (cute - two unique mis-spellings for the same
word :)
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 11/11] Fix error handling in virsh when listing storage volumes
On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
virsh was not checking for a error code when listing storage
volumes. So when listing volumes in a pool that was shutoff,
no output was displayed
* tools/virsh.c: Fix error handling when listing volumes
---
tools/virsh.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index a840758..49dcd6e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -6738,6 +6738,12 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
/* Determine the number of volumes in the pool */
numVolumes = virStoragePoolNumOfVolumes(pool);
+if (numVolumes 0) {
+vshError(ctl, %s, _(Failed to list storage volumes));
+virStoragePoolFree(pool);
+return FALSE;
+}
+
ACK, and not dependent on the rest of the series, if you want to rebase
this and push this early.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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/5] implement public API virDomainIsUpdated
On 11/18/2010 04:20 AM, Osier Yang wrote: Sorry for the trouble of patch names, anyone who would like to push these patches, could you please help update it? s|/4|/5| Actually, git strips [PATCH 2/5] altogether; the resulting patch name would be implement public API virDomainIsUpdated without reference to how long the series was that introduced it. However, is this supposed to be squashed into the 1/4 patch, or is everything still bisectable if this is inserted as a separate patch between 1 and 2 of the original series? [I could find out by manually applying things, but would rather save the time if you have the answer quickly] -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 v2] [TCK] nwfilter: add a test case using concurrency
On 11/18/2010 04:49 AM, Stefan Berger wrote:
V2:
- following Eric's suggestions from review of V1
- all scripts started by the main script trap on signal 2 and clean up
- the main program waits for all child processes to have terminated
- the test now requires each child process to do 10 steps:
- 1 VM start-destroy cycle is 1 step
- 50 filter changes are considered 1 step
- the test terminates after 3 minutes (in case the test system is
very slow / busy) with a bail out message
Now that the existing scripts are cleaned up and my POSIX compliancy
shell-scripting skills have temporarily:-) improved, I am now adding a
test case that exercises concurrency. The test case creates and destroys
2 VMs concurrently as well as changes their referenced filters in a
tight loop. This kind of test previously uncovered
- deadlocks in libvirt due to lock-ordering in the nwfilter subsystem
- libvirt termination bug in libaugeas due to doubly closed file
descriptors and the side effects
All of these have been fixed recently.
The test script is known to run in bash, dash and ksh shells.
They are still Linux-specific (things like date +%s aren't required by
POSIX), but so is the functionality we're testing, so no need to come up
with an alternate timeout method. I only have a couple of trivial nits:
+ # Test runs for a maximum of 3 minutes
+ now=`date +%s`
+ test_end=$(($now + 3 * 60))
+
+ while :;
+ do
+# The logs give us the number of cycles the VMs were created
+# and destroyed.
+val=$(cat ${logvm1} 2/dev/null | tail -n 1)
Useless use of cat. val=$(tail -n1 ${logvm1} 2/dev/null)
+val=$(cat ${logvm2} 2/dev/null | tail -n 1)
Likewise.
ACK with that fixed.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] nwfilter: re-order lock grabbed by IP addr. learn thread
On 11/18/2010 05:16 AM, Stefan Berger wrote: The IP address learning thread was causing a deadlock when it instantiated a filter while a filter update/change was ongoing. The reason for this was the ordering of locks due to the following calls virNWFilterUnlockFilterUpdates() virNWFilterPoolObjFindByName() + * Call this function while holding the NWFilter filter update lock static int __virNWFilterInstantiateFilter(virConnectPtr conn, I'm assuming that's a bogus line in your patch, bool teardownOld, @@ -823,23 +822,30 @@ _virNWFilterInstantiateFilter(virConnect ? net-data.direct.linkdev : NULL; int ifindex; +int rc; if (ifaceGetIndex(true, net-ifname, ifindex)) return 1; ... +virNWFilterLockFilterUpdates(); + +rc = __virNWFilterInstantiateFilter(conn, ...especially given the fact that you grab the lock here, so __virNWFilterInstantiateFilter should NOT have the filter update lock in the caller. ACK, once you fix that compilation error due to the stray line. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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/4] conf: Convert ParseString to use STRPREFIX
On 11/19/2010 09:15 AM, Cole Robinson wrote:
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/util/conf.c | 19 +++
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index ba1a384..a31bbc4 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -394,17 +394,20 @@ virConfParseString(virConfParserCtxtPtr ctxt)
return NULL;
}
NEXT;
-} else if ((ctxt-cur + 6 ctxt-end) (ctxt-cur[0] == '')
- (ctxt-cur[1] == '') (ctxt-cur[2] == '')) {
+} else if ((ctxt-cur + 6 ctxt-end)
+ (STRPREFIX(ctxt-cur, \\\))) {
+/* String starts with python-style triple quotes */
ctxt-cur += 3;
base = ctxt-cur;
-while ((ctxt-cur + 2 ctxt-end) (ctxt-cur[0] == '')
- (ctxt-cur[1] == '') (ctxt-cur[2] == '')) {
How did the old code ever find the closing quotes? If there is anything
except at ctxt-cur after the opening , then the body of the
while loop is never entered and NEXT is never called.
+
+while ((ctxt-cur + 2 ctxt-end)
+ (STRPREFIX(ctxt-cur, \\\))) {
Your rewrite faithfully matches the old code, which means its still
looks buggy.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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/4] conf: Convert ParseString to use STRPREFIX
On 11/19/2010 03:00 PM, Eric Blake wrote:
On 11/19/2010 09:15 AM, Cole Robinson wrote:
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/util/conf.c | 19 +++
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index ba1a384..a31bbc4 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -394,17 +394,20 @@ virConfParseString(virConfParserCtxtPtr ctxt)
return NULL;
}
NEXT;
-} else if ((ctxt-cur + 6 ctxt-end) (ctxt-cur[0] == '')
- (ctxt-cur[1] == '') (ctxt-cur[2] == '')) {
+} else if ((ctxt-cur + 6 ctxt-end)
+ (STRPREFIX(ctxt-cur, \\\))) {
+/* String starts with python-style triple quotes */
ctxt-cur += 3;
base = ctxt-cur;
-while ((ctxt-cur + 2 ctxt-end) (ctxt-cur[0] == '')
- (ctxt-cur[1] == '') (ctxt-cur[2] == '')) {
How did the old code ever find the closing quotes? If there is anything
except at ctxt-cur after the opening , then the body of the
while loop is never entered and NEXT is never called.
Yeah, it didn't work :) See patch 2 for that bug fix, I just didn't want
to refactor and change functionality in 1 patch.
- Cole
+
+while ((ctxt-cur + 2 ctxt-end)
+ (STRPREFIX(ctxt-cur, \\\))) {
Your rewrite faithfully matches the old code, which means its still
looks buggy.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes
On 11/19/2010 09:15 AM, Cole Robinson wrote:
An incorrect check broke matching the closing set of quotes. Update
tests to cover this case for XM config files, and update the domain schema
to allow more path characters.
- param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
+ param
name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param
So far, so good...
/data
/define
define name=devicePath
data type=string
- param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
+ param
name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param
but given that a devicePath can't have '.', should it really be allowed
to have other characters like , , ', , or ?
/data
/define
define name=deviceName
data type=string
- param name=pattern[a-zA-Z0-9_\.\-:/]+/param
+ param
name=pattern[a-zA-Z0-9_\.\-\\amp;quot;apos;lt;gt;:/]+/param
Likewise for deviceName.
+++ b/src/util/conf.c
@@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt)
ctxt-cur += 3;
base = ctxt-cur;
+/* Find the ending triple quotes */
while ((ctxt-cur + 2 ctxt-end)
- (STRPREFIX(ctxt-cur, \\\))) {
+ !(STRPREFIX(ctxt-cur, \\\))) {
Ah, the bug fix for patch 1. ACK to patch 1, then.
+++ b/tests/xmconfigdata/test-escape-paths.cfg
@@ -19,7 +19,7 @@ vnc = 1
vncunused = 1
vnclisten = 127.0.0.1
vncpasswd = 123poi
-disk = [ phy:/dev/HostVG/XenGuest2,hda,w,
file:/root/boot.isotest,hdc:cdrom,r ]
+disk = [ phy:/dev/HostVG/XenGuest2,hda,w,
file:/root/boot.isotest,hdc:cdrom,r,
phy:/dev/HostVG/XenGuest',hdb,w ]
vif = [
mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu
]
parallel = none
serial = none
diff --git a/tests/xmconfigdata/test-escape-paths.xml
b/tests/xmconfigdata/test-escape-paths.xml
index dabf492..13e6e29 100644
--- a/tests/xmconfigdata/test-escape-paths.xml
+++ b/tests/xmconfigdata/test-escape-paths.xml
@@ -31,6 +31,11 @@
target dev='hdc' bus='ide'/
readonly/
/disk
+disk type='block' device='disk'
+ driver name='phy'/
+ source dev='/dev/HostVG/XenGuestapos;quot;'/
Hmm; this really is a case of deviceName in the domain.rng schema. Are
there really devices named with ' or in the name?
If so, then ACK to patch 2. If not, then it would be better to use
disk type='file'source file='/.../XenGuestapos;quot;'/, since that
would pick up on absFilePath, which is more likely to match reality of
having ' or in the name.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] xend: urlencode: Properly escape ''
On 11/19/2010 09:15 AM, Cole Robinson wrote: Since we send the sexpr to xend via HTTP, we need to properly escape '' Signed-off-by: Cole Robinson crobi...@redhat.com --- src/xen/xend_internal.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] xend: Escape reserved sexpr characters
On 11/19/2010 09:15 AM, Cole Robinson wrote:
If we don't escape ' or \ xend can't parse the generated sexpr. This
might over apply the EscapeSexpr routine, but it shouldn't hurt.
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/util/buf.c | 79 +++
src/util/buf.h |1 +
src/xen/xend_internal.c| 115
+++-
tests/xml2sexprdata/xml2sexpr-escape.sexpr |1 +
tests/xml2sexprdata/xml2sexpr-escape.xml | 24 ++
tests/xml2sexprtest.c |1 +
6 files changed, 169 insertions(+), 52 deletions(-)
create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.sexpr
create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.xml
diff --git a/src/util/buf.c b/src/util/buf.c
index 553e2a0..90034ad 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -379,6 +379,85 @@ err:
}
/**
+ * virBufferEscapeSexpr:
+ * @buf: the buffer to dump
+ * @format: a printf like format string but with only one %s parameter
+ * @str: the string argument which need to be escaped
+ *
+ * Do a formatted print with a single string to an sexpr buffer. The string
+ * is escaped to avoid generating a sexpr that xen will choke on. This
+ * doesn't fully escape the sexpr, just enough for our code to work.
+ */
+void
+virBufferEscapeSexpr(const virBufferPtr buf,
+ const char *format,
+ const char *str)
+{
+int size, count, len, grow_size;
+char *escaped, *out;
+const char *cur;
+
+if ((format == NULL) || (buf == NULL) || (str == NULL))
+return;
+
+if (buf-error)
+return;
+
+len = strlen(str);
Right here, is it worth adding:
if (strcspn(str, \\') == len) {
virBufferVSprintf(buf, format, str);
return;
}
+if (VIR_ALLOC_N(escaped, 2 * len + 1) 0) {
+virBufferNoMemory(buf);
+return;
+}
so as to avoid the alloc in the common case of nothing to escape?
+*out = 0;
+
+if ((buf-use = buf-size)
+virBufferGrow(buf, 100) 0) {
+goto err;
+}
That 100 looks wrong; shouldn't it instead be max(100,out-escaped)? For
that matter, why do all this low level stuff; wouldn't it be easier
after *out = 0 to just do:
virBufferVSpring(buf, format, escaped);
VIR_FREE(escaped);
return;
and leave all the resizing and snprintf stuff to the already written
function?
diff --git a/src/util/buf.h b/src/util/buf.h
index 6616898..54f4de5 100644
--- a/src/util/buf.h
@@ -5400,39 +5401,42 @@ xenDaemonFormatSxprDisk(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (hvm) {
/* Xend = 3.0.2 wants a ioemu: prefix on devices for HVM */
-if (xendConfigVersion == 1)
-virBufferVSprintf(buf, (dev 'ioemu:%s'), def-dst);
-else/* But newer does not */
-virBufferVSprintf(buf, (dev '%s:%s'), def-dst,
- def-device == VIR_DOMAIN_DISK_DEVICE_CDROM ?
- cdrom : disk);
+if (xendConfigVersion == 1) {
+virBufferEscapeSexpr(buf, (dev 'ioemu:%s'), def-dst);
+} else {
+/* But newer does not */
+virBufferEscapeSexpr(buf, (dev '%s:, def-dst);
+virBufferEscapeSexpr(buf, %s'),
+ def-device == VIR_DOMAIN_DISK_DEVICE_CDROM
?
+ cdrom : disk);
This can be virBufferVSprintf(buf, %s'),...), given that the only two
possible strings for %s don't need escaping.
@@ -5680,7 +5685,8 @@ xenDaemonFormatSxprSound(virDomainDefPtr def,
def-sounds[i]-model);
return -1;
}
-virBufferVSprintf(buf, %s%s, i ? , : , str);
+virBufferVSprintf(buf, %s, i ? , : );
I'd write this as 'if (i) virBufferAddChar(buf, ',')' rather than a
complicated VSprintf.
diff --git a/tests/xml2sexprdata/xml2sexpr-escape.xml
b/tests/xml2sexprdata/xml2sexpr-escape.xml
new file mode 100644
index 000..73beb6b
--- /dev/null
+++ b/tests/xml2sexprdata/xml2sexpr-escape.xml
@@ -0,0 +1,24 @@
+domain type='xen' id='15'
+ namefvtest/name
+ uuid596a5d2171f48fb2e068e2386a5c413e/uuid
+ os
+typehvm/type
+kernel/var/lib/xen/vmlinuz.2Dn2YT/kernel
+initrd/var/lib/xen/initrd.img.0u-Vhq/initrd
+cmdline
method=http://amp;download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os
/cmdline
Seems unusual to have and in a URL; but the point of this test was
not so much a real configuration as a way to expose the sexpr escaping
code path. Is there any better place to stick this where it won't be
quite so confusing?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
Re: [libvirt] [PATCH 1/3] PHYP: Separating UUID functions in another file
On 11/19/2010 07:55 AM, Eduardo Otubo wrote:
I am moving all the UUID handling functions to phyp_uuid.[ch] files in
order not to bloat the main files phyp_driver.[ch] too much. Doing this
for two reasons:
1) Network management in pHyp does not have a UUID.
2) Need to create another set of functions to manage it.
I also modified some functions to support two types of execution:
DOMAIN and NET, so I can re-use the base common functions.
---
po/POTFILES.in |1 +
src/Makefile.am|3 +-
src/phyp/phyp_driver.c | 464 +-
src/phyp/phyp_driver.h | 41 +++
src/phyp/phyp_uuid.c | 657
src/phyp/phyp_uuid.h | 36 +++
6 files changed, 742 insertions(+), 460 deletions(-)
create mode 100644 src/phyp/phyp_uuid.c
create mode 100644 src/phyp/phyp_uuid.h
[I've rearranged my review a bit; .h before .c]
diff --git a/src/phyp/phyp_uuid.h b/src/phyp/phyp_uuid.h
new file mode 100644
index 000..ddf28f4
--- /dev/null
+++ b/src/phyp/phyp_uuid.h
@@ -0,0 +1,36 @@
+
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright IBM Corp. 2010
+ *
+ * phyp_uuid.c: set of functions to handle lpar uuid and network uuid
+ * which does not have uuid itself, it must be artificially
+ * created.
+ *
...
+
+#include config.h
While there are other counter-examples currently in libvirt.git, the
general rule of thumb tends to be that .c files should include config.h
first before any headers, and therefore .h files should not include it
(because it will already have been included by the .c file including
this .h).
+
+int phypUUIDTable_Init(virConnectPtr conn);
Where is virConnectPtr defined? This header should be self-contained,
rather than relying on the .c file to include pre-requisite headers.
+
+void phypUUIDTable_Free(uuid_tablePtr uuid_table);
Where is uuid_tablePtr defined? Did you mean int? Or should
phypUUIDTable_Init be returning a uuid_tablePtr instead of an int?
+
+int phypUUIDTable_RemLpar(virConnectPtr conn, int id);
+
+int phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid,
int id);
diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h
index bc8e003..603d048 100644
--- a/src/phyp/phyp_driver.h
+++ b/src/phyp/phyp_driver.h
@@ -34,6 +34,7 @@
# define LPAR_EXEC_ERR -1
# define SSH_CONN_ERR -2 /* error while trying to connect to
remote host */
# define SSH_CMD_ERR -3 /* error while trying to execute the
remote cmd */
+# define NETNAME_SIZE 24
Is this adequate? What's your rationale for this size?
typedef struct _ConnectionData ConnectionData;
typedef ConnectionData *ConnectionDataPtr;
@@ -42,6 +43,28 @@ struct _ConnectionData {
int sock;
};
+
+/* This is the network struct that relates
+ * the MAC with UUID generated by the API
+ * */
+typedef struct _net net_t;
+typedef net_t *netPtr;
+struct _net {
+unsigned char uuid[VIR_UUID_BUFLEN];
+long long mac;
Typically a mac is 6 bytes, not 8. Is sign extension going to be a problem?
+char name[NETNAME_SIZE];
+};
+
+/* Struct that holds how many networks we're
+ * handling and a pointer to an array of net structs
+ * */
+typedef struct _uuid_nettable uuid_nettable_t;
+typedef uuid_nettable_t *uuid_nettablePtr;
+struct _uuid_nettable {
+int nnets;
s/int/size_t/
+netPtr *nets;
+};
+
/* This is the lpar (domain) struct that relates
* the ID with UUID generated by the API
* */
@@ -68,6 +91,7 @@ typedef struct _phyp_driver phyp_driver_t;
typedef phyp_driver_t *phyp_driverPtr;
struct _phyp_driver {
uuid_tablePtr uuid_table;
+uuid_nettablePtr uuid_nettable;
virCapsPtr caps;
int vios_id;
@@ -81,4 +105,21 @@ struct _phyp_driver {
int phypRegister(void);
+
+/*
+ * Functions used in the phyp_uuid.c and must be visible outside
phyp_driver.c
+ * */
+int phypNumDomainsGeneric(virConnectPtr conn, unsigned int type);
+
+int phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
+ unsigned int type);
+
+int waitsocket(int socket_fd, LIBSSH2_SESSION * session);
Why is this not in the phyp namespace?
+
+int phypNumOfNetworks(virConnectPtr conn);
+
+int phypListNetworks(virConnectPtr conn, char **const names, int nnames);
+
+int phypListNetworkMAC(virConnectPtr conn, long long *macs, int nnets);
+
#endif /* PHYP_DRIVER_H */
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 4c723a2..6f3f49d 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -59,8 +59,10 @@
#include storage_conf.h
#include nodeinfo.h
#include files.h
+#include network_conf.h
#include phyp_driver.h
+#include phyp_uuid.h
#define VIR_FROM_THIS VIR_FROM_PHYP
@@ -75,7 +77,7 @@
static unsigned const int HMC = 0;
static unsigned const int IVM = 127;
-static int
+int
Re: [libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes
On 11/19/2010 03:38 PM, Eric Blake wrote:
On 11/19/2010 09:15 AM, Cole Robinson wrote:
An incorrect check broke matching the closing set of quotes. Update
tests to cover this case for XM config files, and update the domain schema
to allow more path characters.
- param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
+ param
name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param
So far, so good...
/data
/define
define name=devicePath
data type=string
- param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
+ param
name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param
but given that a devicePath can't have '.', should it really be allowed
to have other characters like , , ', , or ?
I didn't notice the lack of '.' but should probably also be added. From
the XML point of view, a devicePath could really just be any old FS path.
Looking again, deviceName/Path are used in very different ways in the
schema, so this might need additional cleanup anyways. But anything that
represents a path should probably allow all valid path characters,
device or not.
/data
/define
define name=deviceName
data type=string
- param name=pattern[a-zA-Z0-9_\.\-:/]+/param
+ param
name=pattern[a-zA-Z0-9_\.\-\\amp;quot;apos;lt;gt;:/]+/param
Likewise for deviceName.
+++ b/src/util/conf.c
@@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt)
ctxt-cur += 3;
base = ctxt-cur;
+/* Find the ending triple quotes */
while ((ctxt-cur + 2 ctxt-end)
- (STRPREFIX(ctxt-cur, \\\))) {
+ !(STRPREFIX(ctxt-cur, \\\))) {
Ah, the bug fix for patch 1. ACK to patch 1, then.
+++ b/tests/xmconfigdata/test-escape-paths.cfg
@@ -19,7 +19,7 @@ vnc = 1
vncunused = 1
vnclisten = 127.0.0.1
vncpasswd = 123poi
-disk = [ phy:/dev/HostVG/XenGuest2,hda,w,
file:/root/boot.isotest,hdc:cdrom,r ]
+disk = [ phy:/dev/HostVG/XenGuest2,hda,w,
file:/root/boot.isotest,hdc:cdrom,r,
phy:/dev/HostVG/XenGuest',hdb,w ]
vif = [
mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu
]
parallel = none
serial = none
diff --git a/tests/xmconfigdata/test-escape-paths.xml
b/tests/xmconfigdata/test-escape-paths.xml
index dabf492..13e6e29 100644
--- a/tests/xmconfigdata/test-escape-paths.xml
+++ b/tests/xmconfigdata/test-escape-paths.xml
@@ -31,6 +31,11 @@
target dev='hdc' bus='ide'/
readonly/
/disk
+disk type='block' device='disk'
+ driver name='phy'/
+ source dev='/dev/HostVG/XenGuestapos;quot;'/
Hmm; this really is a case of deviceName in the domain.rng schema. Are
there really devices named with ' or in the name?
Probably not, but someone could always create a symbolic link with
whatever valid pathname they want.
If so, then ACK to patch 2. If not, then it would be better to use
disk type='file'source file='/.../XenGuestapos;quot;'/, since that
would pick up on absFilePath, which is more likely to match reality of
having ' or in the name.
Having a whacky named file is probably more likely, but behind the
scenes it's all the same code here that is being tested (xm disk block
generation). The restrictions in RNG were fairly arbitrary anyways.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] PHYP: Adding basic network functions
On 11/19/2010 07:55 AM, Eduardo Otubo wrote:
Now adding some basic operation network functions and its UUID
helpers.
---
src/phyp/phyp_driver.c | 202 ++-
src/phyp/phyp_uuid.c | 177 ++
src/phyp/phyp_uuid.h |8 ++
3 files changed, 382 insertions(+), 5 deletions(-)
+int
+phypListNetworkMAC(virConnectPtr conn, long long *macs, int nnets)
This should probably be static.
+{
+ConnectionData *connection_data = conn-networkPrivateData;
+phyp_driverPtr phyp_driver = conn-privateData;
+LIBSSH2_SESSION *session = connection_data-session;
+int system_type = phyp_driver-system_type;
+char *managed_system = phyp_driver-managed_system;
+int vios_id = phyp_driver-vios_id;
+int exit_status = 0;
+int got = -1;
+char *cmd = NULL;
+char *ret = NULL;
+char *line, *next_line;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(buf, lshwres);
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s, managed_system);
+
+virBufferVSprintf(buf, -r virtualio --rsubtype eth --level lpar
+ |grep -v lpar_id=%d| sed -e 's/^.*mac_addr=//g',
That two-process grep | sed is more efficient as one, and ^ can only
match once, so the g is pointless:
| sed '/lpar_id=%d/d; s/^.*mac_addr=//'
+int
+phypListNetworks(virConnectPtr conn, char **const names, int nnames)
Likewise a candidate for static (in fact, it's sometimes a good idea to
mark everything new as static, see what doesn't compile, and fix the few
fallouts - it's always better to shoot for the smallest possible scope).
+virBufferAddLit(buf, lshwres);
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s, managed_system);
+virBufferVSprintf(buf, -r virtualio --rsubtype slot --level slot
+ |grep eth|grep -v lpar_id=%d|
+ sed -e 's/^.*drc_name=//g', vios_id);
Here, you can replace grep | grep | sed with:
sed -n '/lpar_id=%d/d; /eth/ s/^.*drc_name=//p'
Mostly looks okay, but there's still the issue of using long long
instead of a 6-byte array for MACs that might be worth changing.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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/4] conf: Fix parsing python style triple quotes
On 11/19/2010 02:51 PM, Cole Robinson wrote: On 11/19/2010 03:38 PM, Eric Blake wrote: On 11/19/2010 09:15 AM, Cole Robinson wrote: An incorrect check broke matching the closing set of quotes. Update tests to cover this case for XM config files, and update the domain schema to allow more path characters. - param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param + param name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param So far, so good... /data /define define name=devicePath data type=string - param name=pattern/[a-zA-Z0-9_\+\-/%]+/param + param name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param but given that a devicePath can't have '.', should it really be allowed to have other characters like , , ', , or ? I didn't notice the lack of '.' but should probably also be added. From the XML point of view, a devicePath could really just be any old FS path. If that's the case, then can we consolidate things rather than repeating the same pattern multiple times? That is, can the schema use _just_ filePath and absFilePath, rather than confusing things by adding devicePath? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] PHYP: create, destroy and other network functions
On 11/19/2010 07:55 AM, Eduardo Otubo wrote:
Adding networkCreateXML, networkDestroy, networkIsActive and
networkLookupByName.
In the function phypCreateNetwork I just use the def-domain information to
create
the new network interface because of the behaviour of the HMC and the
hypervisor:
* HMC can't simply create a network interface without assigning it to a
specific
LPAR.
* I also can't assign an IP addr or any other information from the HMC or
VIOS
side, but I can control in which vlan or vswitch it will be attached -
but
thought just in the simplest case scenarion now, I'll make some
improvements
s/scenarion/scenario/
+++ b/src/phyp/phyp_driver.c
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2010 Red Hat, Inc.
- * Copyright IBM Corp. 2009
+ * Copyright IBM Corp. 2010
You should never remove copyright years; this would be okay as 2009-2010.
+
+virBufferAddLit(buf, lshwres );
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s , managed_system);
+
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype slot --level slot
+ -F drc_name,lpar_id|grep %s|
+ sed -e 's/^.*,//g', net-name);
Another grep | sed you can simplify:
sed -n '/%s/ s/^.*,//p'
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype eth --level lpar
+ -F mac_addr,slot_num|grep %lld|
+ sed -e 's/^.*,//g', mac);
MACs are usually represented as hex, not decimal. And if it really is
decimal, wouldn't you want unsigned?
Simplify:
sed -n '/%lld/ /^.*,//p'
+
+if (!def-domain) {
+VIR_ERROR0(_(Domain can't be NULL, you must especify in which
s/especify/specify/
+
+ret = phypExec(session, cmd, exit_status, conn);
+
+if (exit_status 0 || ret != NULL)
+goto err;
+
+/* Need to sleep a little while to wait for the HMC to
+ * complete the execution of the command.
+ * */
+sleep(1);
This seems racy, and 1 second is a long pause. Is there something more
reliable you can use to tell whether HMC is done? Can you set up a
retry loop and sleep for shorter periods of time with retries until it
works to avoid a long pause?
+
+/* Getting the new interface name */
+virBufferAddLit(buf, lshwres );
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s , managed_system);
+
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype slot --level slot
+ |grep lpar_id=%d|grep slot_num=%d|
+ sed -e 's/^.*drc_name=//g', lpar_id, slot);
sed '/lpar_id=%d/!d; /slot_num=%d/!d; s/^.*drc_name=//'
+/* Getting the new interface mac addr */
+virBufferAddLit(buf, lshwres );
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s , managed_system);
+
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype eth --level lpar
+ |grep lpar_id=%d|grep slot_num=%d|
+ sed -e 's/^.*mac_addr=//g', lpar_id, slot);
Similar.
+virBufferAddLit(buf, lshwres );
+if (system_type == HMC)
+virBufferVSprintf(buf, -m %s , managed_system);
+
+virBufferVSprintf(buf,
+ -r virtualio --rsubtype eth --level lpar
+ -F mac_addr,state |grep %lld|
+ sed -e 's/^.*,//g', mac);
Another instance where decimal MAC seems odd.
sed -n '/%lld/ s/^.*,//p'
diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h
index 603d048..34ad84b 100644
--- a/src/phyp/phyp_driver.h
+++ b/src/phyp/phyp_driver.h
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2010 Red Hat, Inc.
- * Copyright IBM Corp. 2009
+ * Copyright IBM Corp. 2010
This hunk seems completely random. Should it be rebased into another
patch that actually touches phyp_driver.h? And should it be 2009-2010?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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/4] Allow probing of image formats without version information
On 11/19/2010 09:18 AM, Adam Litke wrote: Disk image formats that wish to opt-out of version validation are supposed to set versionOffset to -1 in their fileTypeInfo entry. By unconditionally returning False for these formats, virStorageFileMatchesVersion() incorrectly reports a version mismatch when the test was actually skipped. The correct behavior is to return True so these formats can be successfully probed using the magic bytes alone. Signed-off-by: Adam Litke a...@us.ibm.com --- src/util/storage_file.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 4098383..f8ab168 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -478,7 +478,7 @@ virStorageFileMatchesVersion(int format, /* Validate version number info */ if (fileTypeInfo[format].versionOffset == -1) -return false; +return true; ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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/4] QED: Basic support for QED images
On 11/19/2010 09:18 AM, Adam Litke wrote:
Add an entry in fileTypeInfo for QED image files.
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefan.hajno...@uk.ibm.com
Cc: Anthony Liguori aligu...@linux.vnet.ibm.com
---
src/util/storage_file.c |9 -
src/util/storage_file.h |1 +
2 files changed, 9 insertions(+), 1 deletions(-)
+#define QED_HDR_IMAGE_SIZE 40
Given that other formats broke it out by fields, I did likewise for this
one.
+
/* VMDK needs at least this to find backing store,
* other formats need less */
#define STORAGE_MAX_HEAD (20*512)
@@ -151,6 +153,11 @@ static struct FileTypeInfo const fileTypeInfo[] = {
LV_BIG_ENDIAN, 4, 2,
QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW2_HDR_CRYPT, qcow2GetBackingStore,
},
+[VIR_STORAGE_FILE_QED] = {
I'm amending this patch to add a comment to a decent reference URL
(other file types should probably likewise add a reference URL, but that
can come later).
+QED\0, NULL,
+LV_LITTLE_ENDIAN, -1, -1,
+QED_HDR_IMAGE_SIZE, 8, 1, -1, NULL,
Why no backing store extraction function? Not a show-stopper to this
patch, but something to consider adding.
ACK as amended:
diff --git i/src/util/storage_file.c w/src/util/storage_file.c
index 5fe11a3..c011544 100644
--- i/src/util/storage_file.c
+++ w/src/util/storage_file.c
@@ -105,7 +105,7 @@ static int vmdk4GetBackingStore(char **, int *,
#define QCOW2_HDR_EXTENSION_END 0
#define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
-#define QED_HDR_IMAGE_SIZE 40
+#define QED_HDR_IMAGE_SIZE (4+4+4+4+8+8+8)
/* VMDK needs at least this to find backing store,
* other formats need less */
@@ -155,6 +155,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW2_HDR_CRYPT, qcow2GetBackingStore,
},
[VIR_STORAGE_FILE_QED] = {
+/* http://wiki.qemu.org/Features/QED */
QED\0, NULL,
LV_LITTLE_ENDIAN, -1, -1,
QED_HDR_IMAGE_SIZE, 8, 1, -1, NULL,
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] support sheepdog volumes
On 11/18/2010 04:42 PM, MORITA Kazutaka wrote: On 2010/11/18 19:46, Daniel P. Berrange wrote: On Wed, Nov 17, 2010 at 06:19:58PM +0900, MORITA Kazutaka wrote: Sheepdog is a distributed storage system for QEMU. It provides highly available block level storage volumes to VMs like Amazon EBS. This patch adds support for Sheepdog; we can create VMs with sheepdog volumes, and attach sheepdog volumes to running machines via the attach-device command. Sheepdog volumes can be declared like this: disk type='sheepdog' device='disk' driver name='qemu' type='raw' / source vdi='volume_name' host='hostname' port='7000'/ target dev='vda' bus='virtio' / /disk 'host' and 'port' in the source element are optional. If they are not specified, sheepdog clients use the default value (localhost:7000). I'm not too familiar with sheepdog implementation, but I understand that each block device is stored across multiple hosts, but you're only listing one hostname here. In the proposal for supporting RBD/ CEPH in libvirt/QEMU, there's tan option to give multiple hostnames. Is the need to give multiple hostnames going to be something that is also relevant for Sheepdog, or does it cope with this in an entirely different way ? We use the hostname as like a gateway to access sheepdog volumes. Though we could extend it to multiple hostnames to achieve high availability, how to use these parameters as qemu arguments needs to be discussed. For RBD, the monitor hostnames can be set through the environment. It looks like this is easy to add to the libvirt QEMU driver. Would environment variables work for sheepdog as well? As mentioned in the RBD thread[1], I'm wondering whether we should aim for separate 'types' for each network block device or try to merge them into one common syntax like disk type=network device=disk driver name=qemu type=raw / source protocol='rbd|sheepdog|nbd' name=...some image identifier... host name=mon1.example.org port=6000 host name=mon2.example.org port=6000 host name=mon3.example.org port=6000 /source target dev=vda bus=virtio / /disk Are monitor addresses necessary in the above syntax for RBD? I guess the monitor addresses are parameters to create a rbd image, but not ones to use a rbd image. If so, we can make the syntax a bit simpler like The monitors serve as gateways for RBD. All need to be specified so that RBD can still access the cluster if some monitors go down. disk type=network device=disk driver name=qemu type=raw / source protocol='rbd|sheepdog|nbd' name=... host=hostname port=6000 / target dev=vda bus=virtio / /disk Thannks, Kazutaka Thanks, Josh -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] storage_file: Add a new flag to mark backing files that are safe to probe
On 11/19/2010 09:18 AM, Adam Litke wrote: Signed-off-by: Adam Litke a...@us.ibm.com --- src/conf/domain_conf.c |4 src/util/storage_file.c |2 +- src/util/storage_file.h |1 + 3 files changed, 6 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2d11785..a08c846 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7825,6 +7825,10 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (format == VIR_STORAGE_FILE_AUTO !allowProbing) format = VIR_STORAGE_FILE_RAW; /* Stops further recursion */ + +/* Allow probing for image formats that are safe */ +if (format == VIR_STORAGE_FILE_AUTO_SAFE) +format = VIR_STORAGE_FILE_AUTO; } while (nextpath); ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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/4] QED: Basic support for QED images
On Fri, 2010-11-19 at 16:05 -0700, Eric Blake wrote: Why no backing store extraction function? Not a show-stopper to this patch, but something to consider adding. Thanks for the review Eric. Please see patch 4 where I have implemented qedGetBackingStore(). -- Thanks, Adam -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] Support for probing qed image metadata
On 11/19/2010 09:18 AM, Adam Litke wrote:
Implement getBackingStore() for QED images. The header format is defined in
the QED spec: http://wiki.qemu.org/Features/QED .
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefan.hajno...@uk.ibm.com
Cc: Anthony Liguori aligu...@linux.vnet.ibm.com
---
src/util/storage_file.c | 78
++-
1 files changed, 77 insertions(+), 1 deletions(-)
Aha - I should have read this before commenting on patch 2.
@@ -105,6 +107,11 @@ static int vmdk4GetBackingStore(char **, int *,
#define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
#define QED_HDR_IMAGE_SIZE 40
+#define QED_HDR_FEATURES_OFFSET 16
+#define QED_HDR_BACKING_FILE_OFFSET 56
+#define QED_HDR_BACKING_FILE_SIZE 60
+#define QED_F_BACKING_FILE 0x01
+#define QED_F_BACKING_FORMAT_NO_PROBE 0x04
Again, I'll break the offsets into pieces. See below.
+static unsigned long
+qedGetHeaderUL(const unsigned char *loc)
+{
+return ( ((unsigned long)loc[3] 24)
+ | ((unsigned long)loc[2] 16)
+ | ((unsigned long)loc[1] 8)
+ | ((unsigned long)loc[0] 0));
+}
+
+static unsigned long long
+qedGetHeaderULL(const unsigned char *loc)
+{
+return ( ((unsigned long)loc[7] 56)
+ | ((unsigned long)loc[6] 48)
+ | ((unsigned long)loc[5] 40)
+ | ((unsigned long)loc[4] 32)
+ | ((unsigned long)loc[3] 24)
+ | ((unsigned long)loc[2] 16)
+ | ((unsigned long)loc[1] 8)
+ | ((unsigned long)loc[0] 0));
+}
These two routines are independently useful for other little-endian
parsers in the same file. Should we (as a separate patch) rename them
and put them to wider use, as well as adding big-endian counterparts for
the remaining file formats to share? It would cut down on the number of
open-coded integer constructions elsewhere in the file.
+
+static int
+qedGetBackingStore(char **res,
+ int *format,
+ const unsigned char *buf,
+ size_t buf_size)
+{
+unsigned long long flags;
+unsigned long offset, size;
+
+*res = NULL;
+/* Check if this image has a backing file */
+if (buf_size QED_HDR_FEATURES_OFFSET+8)
+return BACKING_STORE_INVALID;
+flags = qedGetHeaderULL(buf + QED_HDR_FEATURES_OFFSET);
+if (!(flags QED_F_BACKING_FILE))
+return BACKING_STORE_OK;
+
+/* Parse the backing file */
+if (buf_size QED_HDR_BACKING_FILE_OFFSET+8)
+return BACKING_STORE_INVALID;
+offset = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_OFFSET);
+if (offset buf_size)
+return BACKING_STORE_INVALID;
+size = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_SIZE);
+if (size == 0)
+return BACKING_STORE_OK;
+if (offset + size buf_size || offset + size offset)
+return BACKING_STORE_INVALID;
+if (size + 1 == 0)
+return BACKING_STORE_INVALID;
This clause is redundant; you already rejected offset + size offset,
and size + 1 == 0 implies size == -1, which would have failed that
earlier check.
ACK, with this squashed in. I've pushed your series.
gdiff --git i/src/util/storage_file.c w/src/util/storage_file.c
index d7b4073..aa117e7 100644
--- i/src/util/storage_file.c
+++ w/src/util/storage_file.c
@@ -107,10 +107,10 @@ qedGetBackingStore(char **, int *, const unsigned
char *, size_t);
#define QCOW2_HDR_EXTENSION_END 0
#define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
-#define QED_HDR_IMAGE_SIZE (4+4+4+4+8+8+8)
-#define QED_HDR_FEATURES_OFFSET 16
-#define QED_HDR_BACKING_FILE_OFFSET 56
-#define QED_HDR_BACKING_FILE_SIZE 60
+#define QED_HDR_FEATURES_OFFSET (4+4+4+4)
+#define QED_HDR_IMAGE_SIZE (QED_HDR_FEATURES_OFFSET+8+8+8)
+#define QED_HDR_BACKING_FILE_OFFSET (QED_HDR_IMAGE_SIZE+8+8)
+#define QED_HDR_BACKING_FILE_SIZE (QED_HDR_BACKING_FILE_OFFSET+4)
#define QED_F_BACKING_FILE 0x01
#define QED_F_BACKING_FORMAT_NO_PROBE 0x04
@@ -475,8 +475,6 @@ qedGetBackingStore(char **res,
return BACKING_STORE_OK;
if (offset + size buf_size || offset + size offset)
return BACKING_STORE_INVALID;
-if (size + 1 == 0)
-return BACKING_STORE_INVALID;
if (VIR_ALLOC_N(*res, size + 1) 0) {
virReportOOMError();
return BACKING_STORE_ERROR;
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] Support for probing qed image metadata
On 11/19/2010 09:18 AM, Adam Litke wrote: Implement getBackingStore() for QED images. The header format is defined in the QED spec: http://wiki.qemu.org/Features/QED . +if (offset + size buf_size || offset + size offset) +return BACKING_STORE_INVALID; As currently coded, buf_size is at most STORAGE_MAX_HEAD (20*512). QED does not appear to have any maximum header size (other than the fact that header size is a multiple of cluster size), and permits a cluster size of 2**26. I don't see anything on the QED file format that requires the backing_filename to occur within the header clusters (that is, shouldn't QED add a file format restriction that backing_filename_offset+backing_filename_size must be less than the start of the first regular cluster?). More worrying, I don't see anything in QED that requires the filename to occur within the first 10K bytes. Do we need to add another enum value to libvirt's backing store callback routine, to be used when the header requests data that lies beyond buf_size but is still feasibly valid, for the case where QED designates a backing store location that is beyond 10k but still before the start of the first cluster, rather than the current approach of just treating it as BACKING_STORE_INVALID? -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 07/10] Remove bogus includes
On 11/18/2010 02:51 AM, Daniel P. Berrange wrote: On Wed, Nov 17, 2010 at 09:28:59PM -0700, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com --- src/conf/domain_conf.c |1 - src/util/hooks.c |1 - 2 files changed, 0 insertions(+), 2 deletions(-) You might want to re-check this one - when I re-tested with macvtap enabled, it caused a build failure. What failure did you see? From my config.log: configure:42373: = configure:42375: configure:42377: Drivers configure:42379: configure:42381: Xen: yes configure:42383: QEMU: yes configure:42385: UML: yes configure:42387: OpenVZ: yes configure:42389: VBox: yes configure:42391: XenAPI: yes configure:42393: LXC: yes configure:42395: PHYP: yes configure:42397: ONE: yes configure:42399: ESX: yes configure:42401: Test: yes configure:42403: Remote: yes configure:42405: Network: yes configure:42407: Libvirtd: yes configure:42409:netcf: yes configure:42411: macvtap: yes configure:42413: virtport: yes so I'm compiling with macvtap, and not seeing any failures. The original patch from you that I amended had one more file touched for a removed include, but that removal did cause compilation failures. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 08/10] Introduce new APIs for spawning processes
On 11/18/2010 02:55 AM, Daniel P. Berrange wrote: On Wed, Nov 17, 2010 at 09:29:00PM -0700, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com This introduces a new set of APIs in src/util/command.h to use for invoking commands. This is intended to replace all current usage of virRun and virExec variants, with a more flexible and less error prone API. My code forgot to ever close() the fds in cmd-preserve. We definitely need todo it in virCommandFree(), but there's a small argument to say we should also do it in virCommandRun/virCommandRunAsync so that if the caller keeps the virCommandPtr alive for a long time, we don't have the open FDs. I'll look into this more. It would also be useful to have a generic API for logging info about the command to an FD (to let us remove that logging code from UML and QEMU LXC drivers). eg +void virCommandWriteArgLog(virCommandPtr cmd, int logfd) Would this be called before virCommandRun and do the logging immediately at that point (or, if something has gone wrong such as allocation failure, then nothing is logged, and the user will eventually learn that nothing was logged when virCommandRun fails), or is it something where you call it once to register the logfd, then calling virCommandRun is what actually writes to the registered fd at the time the child is actually spawned? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: enforce files.h usage
* cfg.mk (sc_prohibit_close): New syntax-check rule.
* src/util/pci.c (pciWaitForDeviceCleanup): Fix violation.
* .x-sc_prohibit_close: New exceptions.
* Makefile.am (EXTRA_DIST): Distribute new file.
---
As promised, here's my followup to Stefan's recent close() cleanups,
which will help us avoid further regressions.
.x-sc_prohibit_close |3 +++
Makefile.am |1 +
cfg.mk |9 +
src/util/pci.c |2 +-
4 files changed, 14 insertions(+), 1 deletions(-)
create mode 100644 .x-sc_prohibit_close
diff --git a/.x-sc_prohibit_close b/.x-sc_prohibit_close
new file mode 100644
index 000..348200c
--- /dev/null
+++ b/.x-sc_prohibit_close
@@ -0,0 +1,3 @@
+^docs/.*
+^HACKING$
+^src/util/files.c$
diff --git a/Makefile.am b/Makefile.am
index d3f8876..bf1b49b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -26,6 +26,7 @@ EXTRA_DIST = \
.x-sc_bindtextdomain \
.x-sc_m4_quote_check \
.x-sc_prohibit_asprintf \
+ .x-sc_prohibit_close \
.x-sc_prohibit_empty_lines_at_EOF \
.x-sc_prohibit_gethostby \
.x-sc_prohibit_gethostname \
diff --git a/cfg.mk b/cfg.mk
index 0851f44..963c7db 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -231,6 +231,15 @@ sc_avoid_write:
halt='consider using safewrite instead of write'\
$(_sc_search_regexp)
+# Avoid functions that can lead to double-close bugs.
+sc_prohibit_close:
+ @prohibit='\[f]close *\(' \
+ halt='use VIR_{FORCE_}[F]CLOSE instead of [f]close' \
+ $(_sc_search_regexp)
+ @prohibit='\fdopen *\('\
+ halt='use VIR_FDOPEN instead of fdopen' \
+ $(_sc_search_regexp)
+
# Similar to the gnulib maint.mk rule for sc_prohibit_strcmp
# Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0.
sc_prohibit_strncmp:
diff --git a/src/util/pci.c b/src/util/pci.c
index bd8c6c5..d38cefa 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1095,7 +1095,7 @@ pciWaitForDeviceCleanup(pciDevice *dev, const char
*matcher)
}
}
-fclose(fp);
+VIR_FORCE_FCLOSE(fp);
return ret;
}
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/11] Stop iSCSI targets automatically logging back in after logout
On 20/11/2010, at 5:45 AM, Eric Blake ebl...@redhat.com wrote: \s and \S are GNU-isms, and regcomp() on other platforms will reject it; is this regex only used on Linux, or do we need to be portable to iscsi implementations on other platforms? As a data point, there are other iSCSI implementations on Linux too, some pretty widely used in some segments. For example, people in the HPC arena, who generally use networking gear that's 10, 20, 40, or 120 Gb/s, use an implementation based upon Linux SCST. It uses different commands to manage, not iscsiadm, and treats LUN 0 differently (from memory). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] [TCK] nwfilter: add a test case using concurrency
On 11/19/2010 02:09 PM, Eric Blake wrote:
On 11/18/2010 04:49 AM, Stefan Berger wrote:
V2:
- following Eric's suggestions from review of V1
- all scripts started by the main script trap on signal 2 and clean up
- the main program waits for all child processes to have terminated
- the test now requires each child process to do 10 steps:
- 1 VM start-destroy cycle is 1 step
- 50 filter changes are considered 1 step
- the test terminates after 3 minutes (in case the test system is
very slow / busy) with a bail out message
Now that the existing scripts are cleaned up and my POSIX compliancy
shell-scripting skills have temporarily:-) improved, I am now adding a
test case that exercises concurrency. The test case creates and destroys
2 VMs concurrently as well as changes their referenced filters in a
tight loop. This kind of test previously uncovered
- deadlocks in libvirt due to lock-ordering in the nwfilter subsystem
- libvirt termination bug in libaugeas due to doubly closed file
descriptors and the side effects
All of these have been fixed recently.
The test script is known to run in bash, dash and ksh shells.
They are still Linux-specific (things like date +%s aren't required by
POSIX), but so is the functionality we're testing, so no need to come up
with an alternate timeout method. I only have a couple of trivial nits:
+ # Test runs for a maximum of 3 minutes
+ now=`date +%s`
+ test_end=$(($now + 3 * 60))
+
+ while :;
+ do
+# The logs give us the number of cycles the VMs were created
+# and destroyed.
+val=$(cat ${logvm1} 2/dev/null | tail -n 1)
Useless use of cat. val=$(tail -n1 ${logvm1} 2/dev/null)
+val=$(cat ${logvm2} 2/dev/null | tail -n 1)
Likewise.
Removed the cat's.
ACK with that fixed.
Pushed.
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: re-order lock grabbed by IP addr. learn thread
On 11/19/2010 02:49 PM, Eric Blake wrote: On 11/18/2010 05:16 AM, Stefan Berger wrote: The IP address learning thread was causing a deadlock when it instantiated a filter while a filter update/change was ongoing. The reason for this was the ordering of locks due to the following calls virNWFilterUnlockFilterUpdates() virNWFilterPoolObjFindByName() + * Call this function while holding the NWFilter filter update lock static int __virNWFilterInstantiateFilter(virConnectPtr conn, I'm assuming that's a bogus line in your patch, Yes, incomplete conversion from // to /* style comment. Fixed. bool teardownOld, @@ -823,23 +822,30 @@ _virNWFilterInstantiateFilter(virConnect ? net-data.direct.linkdev : NULL; int ifindex; +int rc; if (ifaceGetIndex(true, net-ifname,ifindex)) return 1; ... +virNWFilterLockFilterUpdates(); + +rc = __virNWFilterInstantiateFilter(conn, ...especially given the fact that you grab the lock here, so __virNWFilterInstantiateFilter should NOT have the filter update lock in the caller. ACK, once you fix that compilation error due to the stray line. Pushed. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] implement public API virDomainIsUpdated
于 2010年11月20日 02:57, Eric Blake 写道: On 11/18/2010 04:20 AM, Osier Yang wrote: Sorry for the trouble of patch names, anyone who would like to push these patches, could you please help update it? s|/4|/5| Actually, git strips [PATCH 2/5] altogether; the resulting patch name would be implement public API virDomainIsUpdated without reference to how long the series was that introduced it. good to known it.. However, is this supposed to be squashed into the 1/4 patch, or is everything still bisectable if this is inserted as a separate patch between 1 and 2 of the original series? It's the separate patch, and should be inserted between 1 and 2. Thanks. - Osier [I could find out by manually applying things, but would rather save the time if you have the answer quickly] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] allow '#' as valid character for domain name
于 2010年11月20日 02:00, Laine Stump 写道:
On 11/19/2010 05:29 AM, Osier Yang wrote:
* docs/schemas/domain.rng
---
docs/schemas/domain.rng | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index bbbc846..815134d 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -2003,7 +2003,7 @@
/define
define name=domainName
data type=string
-param name=pattern[A-Za-z0-9_\.\+\-amp;:/]+/param
+param name=pattern[A-Za-z0-9_\.\+\-\\#amp;:/]+/param
/data
/define
define name=diskSerial
What's your motivation for this?
If domain.rng is used similarly to the other .rng files I'm more
familiar with, it's only actually examined during the tests run as part
of make check, so it won't have any effect on actual operation. Is
this what you intended?
# rpm -qf /usr/bin/virt-xml-validate
libvirt-client-0.8.3-2.fc14.x86_64
# rpm -ql libvirt-client | grep 'domain.rng'
/usr/share/libvirt/schemas/domain.rng
# find . -type f -name domain.rng
./docs/schemas/domain.rng
# strings /usr/bin/virt-xml-validate | grep rng
SCHEMA=/usr/share/libvirt/schemas/${TYPE}.rng
It's used by virt-xml-validate, I'm sure also could find more proofs
in Makefile and source code, but think it's no need.
# seems like a problematic character to put in a domain name - for
example it would need to be escaped or quoted if it was ever on a
commandline - what happens when that name gets passed to qemu, for
example? Or a user-written shell script that calls virsh? Also,
virt-manager doesn't allow it.
yes, virt-manager doesn't allow it indeed, and also it will also
need to be escaped in shell.
However, we allow # as domain name in virsh, actually we have
two bugs, one says # should not be allowed, the other says
it should be allowed, yeah, they are conflicts.
Though I prefer disallowing it, intended to send the patch so
that get more ideas from guys, and could have a consistent conclusion,
probly we should also have a common rule for domain name as
documentation, but diffrent driver accepts diffrent character set,
that's the problem.
Any idea?
Regards
- Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] allow '#' as valid character for domain name
于 2010年11月20日 02:41, Eric Blake 写道: On 11/19/2010 11:00 AM, Laine Stump wrote: -param name=pattern[A-Za-z0-9_\.\+\-amp;:/]+/param +param name=pattern[A-Za-z0-9_\.\+\-\\#amp;:/]+/param /data /define define name=diskSerial What's your motivation for this? If domain.rng is used similarly to the other .rng files I'm more familiar with, it's only actually examined during the tests run as part of make check, so it won't have any effect on actual operation. Is this what you intended? Additionally, it would be a good idea to add a .xml file somewhere in the tests/ hierarchy that has such a domain name, as a way of getting test exposure of both the RelaxNG schema validation and of actual XML handling in libvirt itself. yeah, agree, we have conflicts between RelaxNG schema validation and XML allowed by libvirt. # seems like a problematic character to put in a domain name - for example it would need to be escaped or quoted if it was ever on a commandline Only if it is the first character of the domain name, but yes, this aspect alone makes me need to see a concrete example of someone using a domain like this before we can add support for it. Rich pasted his domain list in IRC yesterday, the names not only contains #, but also |.. etc. :-) $ echo #ab $ echo a#b a#b -- 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
