[Libvir] [PATCH] Add /usr/sbin to path when searching for iptables
iptables resides in /usr/sbin on SuSE distros. Add it to path when searching for iptables. Regards, Jim diff -ur a/configure.in b/configure.in --- a/configure.in 2008-05-05 13:46:20.0 -0600 +++ b/configure.in 2008-05-05 13:43:14.0 -0600 @@ -217,7 +217,7 @@ AC_DEFINE_UNQUOTED(LOKKIT_PATH, $LOKKIT_PATH, [path to lokkit binary]) fi -AC_PATH_PROG(IPTABLES_PATH, iptables, /sbin/iptables) +AC_PATH_PROG(IPTABLES_PATH, iptables, /sbin/iptables, [/usr/sbin:$PATH]) AC_DEFINE_UNQUOTED(IPTABLES_PATH, $IPTABLES_PATH, [path to iptables binary]) dnl -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH]Fix minor issues in logical storage backend
Hi All!
I came across some problems trying to create a new LVM-based storage
pool using this config
pool type=logical
nametest_vg/name
source
device path=/dev/sdb1/
/source
target
path/dev/test_vg/path
/target
/pool
Volume group did not previously exist so I did
virsh pool-define above.xml
virsh pool-build test_vg
pool-build failed since the backend logical storage driver does not have
VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE set in flags. Without this flag
set, the device element is never parsed in virStoragePoolDefParseDoc()
(storage_conf.c), causing pvcreate to fail since no physical volume is
specified.
After this problem was memory corruption cause by miscalculating the
size of vgcreate command line :-).
This patch fixes both issues.
Regards,
Jim
Index: src/storage_backend_logical.c
===
RCS file: /data/cvs/libvirt/src/storage_backend_logical.c,v
retrieving revision 1.11
diff -u -r1.11 storage_backend_logical.c
--- src/storage_backend_logical.c 27 Aug 2008 20:05:59 - 1.11
+++ src/storage_backend_logical.c 28 Aug 2008 23:58:54 -
@@ -351,7 +351,7 @@
memset(zeros, 0, sizeof(zeros));
/* XXX multiple pvs */
-if (VIR_ALLOC_N(vgargv, 1) 0) {
+if (VIR_ALLOC_N(vgargv, 3 + pool-def-source.ndevice) 0) {
virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(command line));
return -1;
}
@@ -618,6 +618,7 @@
.deleteVol = virStorageBackendLogicalDeleteVol,
.poolOptions = {
+.flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE),
.formatFromString = virStorageBackendLogicalPoolFormatFromString,
.formatToString = virStorageBackendLogicalPoolFormatToString,
},
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] Request for additional entry points
Daniel Veillard wrote: On Fri, Mar 31, 2006 at 04:47:38PM -0500, Daniel Veillard wrote: I just added the following: /** * virDomainReboot: * @domain: a domain object * @flags: extra flags for the reboot operation, not used yet * * Reboot a domain, the domain object is still usable there after but * the domain OS is being stopped for a restart. * Note that the guest OS may ignore the request. * * Returns 0 in case of success and -1 in case of failure. */ int virDomainReboot(virDomainPtr domain, unsigned int flags) there is also a new virsh reboot command to test it. However there when trying it I saw that the guest OS shutdown in reboot mode: Turning off quotas: Unmounting pipe file systems: Please stand by while rebooting the system... md: stopping all md devices. md: md0 switched to read-only mode. Restarting system. . but the domain was not relaunched by default. I guess this also requires to set up specific xenstore attributes so that the domain is actually restarted. So not completely done yet. You are probably aware of this but FYI... I used virDomainReboot() with success when the domain was created with xm create and on_reboot = 'restart' in domain config file. Looking at virDomainParseXMLDesc() in xml.c I see we are not accounting for a on_reboot element (or on_crash, on_poweroff) in the XML. Are you working on this Daniel or should I take a stab at it? I'm guessing you just want elements like on_rebootrestart/on_reboot on_poweroffdestroy/on_poweroff on_crashrestart/on_crash Same goes turning sxp to XML in xend_parse_sexp_desc() in xend_internal.c. Regards, Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[Libvir] Re: [Xen-cim] LTC provider working on SLES/openwbem
Daniel Veillard wrote:
On Thu, Apr 20, 2006 at 04:24:28PM -0600, Jim Fehlig wrote:
FYI,
I have managed to get one of the LTC providers (Xen_ComputerSystem)
working on SLES using openwbem cimom, the xm shim, and libvirt. The
following changes were made:
libvirt
- changed xend_parse_sexp_desc() to not fail if kernel not found. sexp
from xend does not contain kernel for domain 0.
Makes sense, could you send that patch ?
thanks,
Daniel
Attached.
Jim
Index: xend_internal.c
===
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.24
diff -u -r1.24 xend_internal.c
--- xend_internal.c 20 Apr 2006 14:28:01 - 1.24
+++ xend_internal.c 21 Apr 2006 18:02:22 -
@@ -1350,28 +1350,30 @@
tmp = sexpr_node(root, domain/bootloader);
if (tmp != NULL)
virBufferVSprintf(buf, bootloader%s/bootloader\n, tmp);
-tmp = sexpr_node(root, domain/image/linux/kernel);
-if (tmp == NULL) {
-/*
- * TODO: we will need some fallback here for other guest OSes
- */
-virXendError(NULL, VIR_ERR_INTERNAL_ERROR,
- domain informations incomplete, missing kernel);
-goto error;
+if (sexpr_lookup(root, domain/image)) {
+tmp = sexpr_node(root, domain/image/linux/kernel);
+if (tmp == NULL) {
+ /*
+* TODO: we will need some fallback here for other guest OSes
+*/
+ virXendError(NULL, VIR_ERR_INTERNAL_ERROR,
+domain informations incomplete, missing kernel);
+ goto error;
+}
+virBufferAdd(buf, os\n, 7);
+virBufferVSprintf(buf, typelinux/type\n);
+virBufferVSprintf(buf, kernel%s/kernel\n, tmp);
+tmp = sexpr_node(root, domain/image/linux/ramdisk);
+if ((tmp != NULL) (tmp[0] != 0))
+ virBufferVSprintf(buf, initrd%s/initrd\n, tmp);
+tmp = sexpr_node(root, domain/image/linux/root);
+if ((tmp != NULL) (tmp[0] != 0))
+ virBufferVSprintf(buf, root%s/root\n, tmp);
+tmp = sexpr_node(root, domain/image/linux/args);
+if ((tmp != NULL) (tmp[0] != 0))
+ virBufferVSprintf(buf, cmdline%s/cmdline\n, tmp);
+virBufferAdd(buf, /os\n, 8);
}
-virBufferAdd(buf, os\n, 7);
-virBufferVSprintf(buf, typelinux/type\n);
-virBufferVSprintf(buf, kernel%s/kernel\n, tmp);
-tmp = sexpr_node(root, domain/image/linux/ramdisk);
-if ((tmp != NULL) (tmp[0] != 0))
-virBufferVSprintf(buf, initrd%s/initrd\n, tmp);
-tmp = sexpr_node(root, domain/image/linux/root);
-if ((tmp != NULL) (tmp[0] != 0))
-virBufferVSprintf(buf, root%s/root\n, tmp);
-tmp = sexpr_node(root, domain/image/linux/args);
-if ((tmp != NULL) (tmp[0] != 0))
-virBufferVSprintf(buf, cmdline%s/cmdline\n, tmp);
-virBufferAdd(buf, /os\n, 8);
virBufferVSprintf(buf, memory%d/memory\n,
(int) (sexpr_u64(root, domain/maxmem) 10));
virBufferVSprintf(buf, vcpu%d/vcpu\n,
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] Request for additional entry points
Daniel P. Berrange wrote: On Tue, Apr 18, 2006 at 05:40:07PM -0400, Daniel Veillard wrote: On Tue, Apr 18, 2006 at 04:40:13PM -0400, Daniel Veillard wrote: So we have 2 more APIs which allows to define the XML for a domain and name it. That then allow to reserve that name, and the domain may be started later with a simpler API. Since I have troubles understanding why you have such an issue with this, let's try to be as clear as possible. What I would expect is the following APIs to be added: /* define a domain, but does not start it */ virDomainPtr virDomainDefineXML(virConnectPtr conn, const char *xml); /* undefine a domain but does not stop it if running */ intvirDomainUndefine(virDomainPtr domain); /* list the defined domains */ int virConnectListDefinedDomains(virConnectPtr conn, const char **names, int maxnames); /* launch a defined domain */ int virDomainCreate(virDomainPtr domain); What would you anticipate the scope of the domains defined with these APIs to be. Would they be visible to only to the app defining them (ie just kept in process memory), shared between any locally running app using libvirt on the host, or shared between arbitrary apps connecting to the HV, even if connecting remotely ? extensions to the current behaviour: - new state for defined non-running domains showing in virNodeGetInfo - virDomainLookupByName() could return a defined non-running domain - virDomainCreateLinux() would fail if a domain with the same name is already defined - a number of existing APIs would fail on defined but non-running domains. that's it. Now what is fundamentally wrong with that ? You don't have to use it if you don't need it I assume the problem is harder than this. There is nothing fundamentally wrong - *if* you are only aiming to support the needs of a simple local management tool. In the broader case though it does not look to be effective because it: - Pre-supposes that there is a relation between a passive VM and a particular host. This may be true in the simple local case, of VMWare Workstation type tool, but in a distributed environment it is just doesn't make any sense - you have a group of passives VMs and a group of potential hosts to run them on - there is no 1-to-1 mapping between passive domains hosts. - Pre-supposes that the application creating/editing the passive VM configurations wants to store them in the libvirt XML format. Again while this may be true in the basic case it doesn't bear up to more interesting scenarios, such as the idea of VM templates. In such a case a generic template would define # of CPUs, # disk adapters, and other general VM capabilities. A passive VM would only maintain perhaps its name, path to disk image a template name. - Assumes that passive domains are concept which neccessarily even exist a head of time. A web server farm may merely comprise a stateless OS image, and a set of potential hosts - domains for new OS images are defined on the fly as demand requires. Now I know there is nothing that requires an application to make use of these APIs - one could simply call virDomainCreate(const char *xml) at the time the new domain is needed, but in doing so there are now a broad set of scenarios where 'virConnectListDefinedDomains' will return an empty list. So I think my core question is - what are the client application uses cases scenarios which these APIs are intended to serve. If the app use cases / scenario were clearly described, then it could well be the case that the points I raise above are completely irrelevant. But without this info on uses cases I can't say whether the APIs described are sufficiently flexible or not. The DMTF System Virtualization Profile describes the idea of a defined computer system, which is a computer system residing at the hosting node in the disabled, stopped, or off state but which can be immediately activatable. The computer system's image, disks, etc are available and ready to be consumed at the hosting node. The requested APIs were an attempt to represent the defined state (a computer system that is ready to go, just needs to be activated) further down the stack as I would think this state would be useful for other management apps and models. Certainly in more interesting management scenarios, templates, config, images, etc. could be stored in other places and deployed to a selected host. However, it seems valid that the management app may also want to query hosts about previously deployed but inactive domains. E.g. a possible use case: - Management app user selects a VM for deployment and an appropriate target host. - Management app queries target host to see if it knows anything about the VM - finds a defined VM already at the host - ensure the config at host matches the centrally maintained config description - activate the VM - find
Re: [Libvir] Re: Proposal : add 3 functions to Libvirt API, for virtual CPUs
Has any progress been made on this proposal? I don't see anything in current CVS. This functionality would be useful for Xen-CIM project. Regards, Jim [EMAIL PROTECTED] wrote: Corrections on proposal: 1) PinVcpus Replace: * @cpumap: pointer to a bit map of real CPUs (format in virVcpuInfo) * @maplen: length of cpumap, in 8-bit bytes by: * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes). *Each bit set to 1 means that corresponding CPU is usable. *Bytes are stored in little-endian order: CPU0-7, 8-15... *In each byte, lowest CPU number is least significant bit. * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in *underlying virtualization system (Xen...). *If maplen size, missing bytes are set to zero. *If maplen size, failure code is returned. 2) GetVcpu Add 4rth argument: * @maplen: number of bytes in cpumap field of virVcpuInfo virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, int maplen) 3) Structure VcpuInfo Suppress: #define VIR_MAX_CPUS256 Replace: unsigned char cpumap[VIR_MAX_CPUS/8]; /* Bit map of usable real CPUs. by: unsigned char cpumap[];/* Bit map of usable real CPUs. Variable length: it may be less or greater than size of CPU map in underlying virtualization system (Xen...). 4) Accessor macros: to be defined later. Veuillez répondre à [EMAIL PROTECTED] Pour :[EMAIL PROTECTED] cc :libvir-list@redhat.com Objet :Re: Proposal : add 3 functions to Libvirt API, for virtual CPUs On Fri, Jun 30, 2006 at 04:00:45PM +0200, [EMAIL PROTECTED] wrote: For our administration, we need the following actions, while concerned domain is running: 1) Change the number of virtual CPUs. 2) Change the pinning (affinity) of a virtual CPU on real CPUs. 3) Get detailed information for each virtual CPU. Currently there is no Libvirt function provided for that. We suggest to add the following 3 functions (in libvirt.c): /** * virDomainSetVcpus: * @domain: pointer to domain object, or NULL for Domain0 * @nvcpus: the new number of virtual CPUs for this domain * * Dynamically change the number of virtual CPUs used by the domain. * Note that this call may fail if the underlying virtualization hypervisor * does not support it or if growing the number is arbitrary limited. * This function requires priviledged access to the hypervisor. * * Returns 0 in case of success, -1 in case of failure. */ int virDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus) okay /** * virDomainPinVcpu: * @domain: pointer to domain object, or NULL for Domain0 * @vcpu: virtual CPU number * @cpumap: pointer to a bit map of real CPUs (format in virVcpuInfo) * @maplen: length of cpumap, in 8-bit bytes * * Dynamically change the real CPUs which can be allocated to a virtual CPU. * This function requires priviledged access to the hypervisor. * * Returns 0 in case of success, -1 in case of failure. */ int virDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, unsigned char *cpumap, int maplen) Can you explain more clearly what is the format of cpumap ? An example would be welcome, and that would be needed for the doc and maybe testing. What would happen if maplen is or than the number of CPU divided by 8 ? /** * virDomainGetVcpus: * @domain: pointer to domain object, or NULL for Domain0 * @info: pointer to an array of virVcpuInfo structures * @maxinfo: number of structures in info array * * Extract information about virtual CPUs of a domain, store it in info array. * * Returns the number of info filled in case of success, -1 in case of failure. */ int virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo) Hum ... now the problem with that API entry point is that we 'burn' the maximum 256 processors in the ABI, i.e. if we ever need to go past 256 client and servers need to be recompiled. Maybe this is not a real problem in practice but that's annoying. Is there existing APIs doing this kind of things (in POSIX for example), and what hard limit did they use ? Maybe int virDomainGetVcpusNr(virDomainPtr domain, int nr, virVcpuInfoPtr info, int maxCPU); Where the maxCPU is defined by the client as the number of real CPU it defined in its virVcpuInfoPtr and then an iteration over the virtual CPU defined in the domain is possible too. Of course if the domain uses many virtual CPUs this would become expensive but somehow I don't see that being the common use, I would rather guess the domains created use a few CPUs even if instantiated on a very large machine. This with the following structure (in libvirt.h): /** * virVcpuInfo: structure for information about a virtual CPU in a domain. */ #define VIR_MAX_CPUS256 Hum, there is already NUMA machines with more than 256 processors, it's
Re: [Libvir] Re: Virtual CPU functions
Philippe Berthault wrote: Daniel Veillard a écrit : On Tue, Aug 01, 2006 at 05:06:59PM +0200, Philippe Berthault wrote: [snip] I wonder how people are most likely to use those APIs. Building scenarios like: - physical CPU is to be locked to serve only VCPU N in domain D - domain A and domain B should be served by disjoint physical CPUs sets - monitoring are the most common uses I would guess but I may be wrong. First would require: - virDomainPinVcpu I guess Second would require: - virDomainGetCpus and a number of calls to limit to sets :-\ The last one is likely to require getting full maps, and since it is likely to be called frequently the cheapest the better If people who expressed interest on the list about VCPU function could express their principal use case it may help getting the APIs right. About third point (monitoring): I think the virDomainGetVcpus API isn't adequate for this purpose. It would be better to have an API (to be defined) which give the state of all physical CPUs because it's the hardware resources we need to monitor, not the virtual ones. The virDomainGetVcpus API permits to obtain the relation vcpu-physical_cpu(s) but for monitoring usage, it's not interesting. It would be better to have an API which give the reverse relation : physical_cpu-vpcu(s) independently of domains and give the physical CPU usage. With the virDomainGetVcpus API, it's impossible to determine if a physical cpu is underused or overused and it's this information we need to know for monitoring and for load-balancing. I agree and do not see a monitoring use case for these APIs, only setting/getting configuration. Perhaps we need more host-side entry points, for monitoring host resources? Regards, Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[Libvir] Problem with hypercall
Stumbled across a problem trying to list domains with libvirt-0.1.5. Using virsh I get errors such as xen81:/tests/jim # virsh list Id Name State -- 0 Domain-0 running libvir: Xen Daemon error : GET operation failed: No such domain 16 xm shows xen81:/tests/jim # xm list Name ID Mem(MiB) VCPUs State Time(s) Domain-0 0 2048 4 r- 815825.8 vm11 512 2 -b 0.3 I found that the buffer provided for XEN_V1_OP_GETDOMAININFOLIST hypercall differs slightly from the buffer in xen/dom0_ops.h. Attached is a patch against current cvs that works for me, but I'm not familiar with this part of the code so not sure if this is the proper fix. I'm using xen 3.0.2 that shipped with SLES10. Regards, Jim Index: xen_internal.c === RCS file: /data/cvs/libvirt/src/xen_internal.c,v retrieving revision 1.39 diff -u -r1.39 xen_internal.c --- xen_internal.c 5 Sep 2006 06:48:44 - 1.39 +++ xen_internal.c 13 Sep 2006 19:36:25 - @@ -90,7 +90,7 @@ uint32_t flags; /* falgs, see before */ uint64_t tot_pages; /* total number of pages used */ uint64_t max_pages; /* maximum number of pages allowed */ -uint64_t shared_info_frame; /* MFN of shared_info struct */ +unsigned long shared_info_frame; /* MFN of shared_info struct */ uint64_t cpu_time; /* CPU time used */ uint32_t nr_online_vcpus; /* Number of VCPUs currently online. */ uint32_t max_vcpu_id; /* Maximum VCPUID in use by this domain. */ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] PATCH: Allow remote driver to handle any connection URI
Daniel P. Berrange wrote: We currently have logic in the remote driver so that it handles the local QEMU driver URIs, so they get re-directed to the daemon. It also handles networking APIs for Xen driver. For normal APIs, Xen has the auto-spawned setuid proxy daemon. This was very useful at the time we wrote it, but it only supports a handful of operations, and only in read-only mode. One other factor is that SUSE, for example, do not ship it because it is setuid. I don't know whether this is just a general policy, or just because they've not had time to audit it, but that's not very good for their users. Yep. Reason is the former. But this can be overridden (followed by an audit) if there is a good case. Apparently my case wasn't strong enough. Too be fair though, I didn't push hard. And now that I've seen this mail I'm reminded that I wanted to push this for openSUSE 10.3 -- which went GM today :-(. Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Xen: Support cpu_weight and cpu_cap for Xen.
David Lutterkort wrote: On Fri, 2007-10-26 at 09:08 -0400, Daniel Veillard wrote: I can understand the need to make it easy for an user, I still don't think this means those tuning informations need to be associated to the domain definition, to me it is somehow orthogonal to the domain themselves and I would rather try to provide a good solution to the problem, than try to imitate how Xen was doing that. Surely, users who set tuning parameters view them as part of their domain description. I agree. It appears the discussions around recent Linux-VServer patches includes tuning information in the XML description so I'm a little confused if this idea is being received positively or not. I would like to avoid calling out to some other API for scheduling parameters at domU definition if at all possible. Perhaps I've been away too long and this functionality already exists :-), but not seeing it in http://www.libvirt.org/format.html Regards, Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] Libvirt version
Katti, Vadiraj (STSD-Openview) wrote: Hi, Currently the latest version of libvirt available is 0.3.3. But SLES 10 ships 0.2.0 version of libvirt (not sure what version RHEL 5.1 ships). Any idea when and on what basis SLES decides to ship the upgraded version of libvirt. libvirt will be updated in SLES10 SP2. You can always use openSUSE build service to build rpms for SUSE and other distros: https://build.opensuse.org/ I have latest libvirt packages for 10.3 and SLE in my home project: https://build.opensuse.org/package/show?project=home%3Ajfehligpackage=libvirt Well, guess 'latest' isn't quite true with recently released 0.4.0 :-). I'll update 'soon'. Regards, Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] Libvirt version
Jim Fehlig wrote: [...] I have latest libvirt packages for 10.3 and SLE in my home project: https://build.opensuse.org/package/show?project=home%3Ajfehligpackage=libvirt Oops, sorry - I was logged in when I copied that link. For SLE you can get rpms here http://download.opensuse.org/repositories/home:/jfehlig/SLE_10/ Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix build after commit e3ba4025
This patch fixes some build error I've encountered, but would prefer an
ACK before pushing.
Thanks,
Jim
From ff81637b66793fd4b2b46e8e04357e8d406dde31 Mon Sep 17 00:00:00 2001
From: Jim Fehlig jfeh...@suse.com
Date: Mon, 5 Mar 2012 12:08:54 -0700
Subject: [PATCH] Fix build after commit e3ba4025
Commit e3ba4025 introduced a few build errors with HAVE_LIBNL undefined.
---
src/util/virnetdevvportprofile.c |2 +-
src/util/virnetlink.c| 10 ++
src/util/virnetlink.h|2 +-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 00fd123..f6db292 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -1071,7 +1071,7 @@ int virNetDevVPortProfileAssociate(const char *macvtap_ifname ATTRIBUTE_UNUSED,
const char *linkdev ATTRIBUTE_UNUSED,
const unsigned char *vmuuid ATTRIBUTE_UNUSED,
enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED,
- bool setlink_only)
+ bool setlink_only ATTRIBUTE_UNUSED)
{
virReportSystemError(ENOSYS, %s,
_(Virtual port profile association not supported on this platform));
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 264fcdb..63d 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -585,9 +585,10 @@ int virNetlinkEventServiceIsRunning(void)
* virNetlinkEventAddClient: register a callback for handling of
* netlink messages
*/
-int virNetlinkEventAddClient(virNetlinkEventHandleCallback cb,
- virNetlinkEventRemoveCallback cb,
- void *opaque, const unsigned char *macaddr)
+int virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB ATTRIBUTE_UNUSED,
+ virNetlinkEventRemoveCallback removeCB ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED,
+ const unsigned char *macaddr ATTRIBUTE_UNUSED)
{
netlinkError(VIR_ERR_INTERNAL_ERROR,
%s,
@@ -602,7 +603,8 @@ int virNetlinkEventAddClient(virNetlinkEventHandleCallback cb,
/**
* virNetlinkEventRemoveClient: unregister a callback from a netlink monitor
*/
-int virNetlinkEventRemoveClient(int watch, const unsigned char *macaddr)
+int virNetlinkEventRemoveClient(int watch ATTRIBUTE_UNUSED,
+const unsigned char *macaddr ATTRIBUTE_UNUSED)
{
netlinkError(VIR_ERR_INTERNAL_ERROR,
%s,
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 75533a3..71c962e 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -55,7 +55,7 @@ int virNetlinkEventServiceStart(void);
/**
* virNetlinkEventServiceIsRunning: returns if the netlink event service is running.
*/
-bool virNetlinkEventServiceIsRunning(void);
+int virNetlinkEventServiceIsRunning(void);
/**
* virNetlinkEventAddClient: register a callback for handling of netlink messages
--
1.7.8.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix build after commit e3ba4025
Eric Blake wrote: On 03/05/2012 12:17 PM, Jim Fehlig wrote: +++ b/src/util/virnetlink.h @@ -55,7 +55,7 @@ int virNetlinkEventServiceStart(void); /** * virNetlinkEventServiceIsRunning: returns if the netlink event service is running. */ -bool virNetlinkEventServiceIsRunning(void); +int virNetlinkEventServiceIsRunning(void); But this hunk looks wrong. Yes, it is wrong. I changed the definition in virnetlink.c, as it should have been, and pushed the result. Thanks, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote:
Add migration APIs for libxl driver.
Implemented in migration version 3. Based on xen 4.1.
I didn't get a chance to test this yet, but have some initial review
comments.
Signed-off-by: Chunyan Liu cy...@suse.com
---
src/libxl/libxl_driver.c | 617
++
src/libxl/libxl_driver.h | 17 ++-
2 files changed, 632 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d5fa64a..4037635 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -30,6 +30,12 @@
#include math.h
#include libxl.h
#include fcntl.h
+#include stdlib.h
+#include string.h
+#include sys/types.h
+#include sys/socket.h
+#include arpa/inet.h
+#include netdb.h
#include internal.h
#include logging.h
@@ -60,6 +66,13 @@
#define XEN_SCHED_CREDIT_NPARAM 2
static libxlDriverPrivatePtr libxl_driver = NULL;
+static virThread migrate_receive_thread;
This prevents receiving concurrent migrations.
+
+typedef struct migrate_receive_args {
+virConnectPtr conn;
+virDomainObjPtr vm;
+int sockfd;
+} migrate_receive_args;
If there is a future need to extend this structure, will it cause
incompatibility issues between a source with the extensions and a
destination without? Or vise versa?
/* Function declarations */
static int
@@ -1095,6 +1108,17 @@ libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED)
return 0;
}
+static int
+libxlSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
+{
+switch (feature) {
+case VIR_DRV_FEATURE_MIGRATION_V3:
+return 1;
+default:
+return 0;
+}
+}
+
static const char *
libxlGetType(virConnectPtr conn ATTRIBUTE_UNUSED)
{
@@ -3842,12 +3866,600 @@ libxlIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED)
return 1;
}
+static int libxlReadFixedMessage(int fd, const void *msg, int msgsz)
+{
+char buf[msgsz];
+
+if (saferead(fd, buf, msgsz) != msgsz || memcmp(buf, msg, msgsz)) {
+return -1;
+}
+
+return 0;
+}
+
+static int doParseURI(const char *uri, char **p_hostname, int *p_port)
+{
+char *p, *hostname;
+char port[16] = 0;
+
+if (strstr (uri, //)) { /* Full URI. */
+virURIPtr uriptr = virURIParse(uri);
+if (!uriptr) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(Invalid URI));
+return -1;
+}
+if (uriptr-scheme STRCASENEQ(uriptr-scheme, xlmigr)) {
+libxlError(VIR_ERR_INVALID_ARG,
+_(Only xlmigr://
+ migrations are supported by Xen));
+xmlFreeURI(uriptr);
+return -1;
+}
This is just tcp migration. Any reason for requiring the xlmigr scheme?
+if (!uriptr-server) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(A hostname must be
+ specified in the URI));
+xmlFreeURI(uriptr);
+return -1;
+}
+hostname = strdup(uriptr-server);
I think it would be better to rework this, and other uses of strdup()
and snprintf() in this function, to use virAsprintf().
+if (!hostname) {
+virReportOOMError();
+xmlFreeURI(uriptr);
+return -1;
+}
+if (uriptr-port)
+snprintf(port, sizeof(port), %d, uriptr-port);
+xmlFreeURI(uriptr);
+}
+else if ((p = strrchr(uri, ':')) != NULL) { /* hostname:port */
+int port_nr, n;
+
+if (virStrToLong_i(p+1, NULL, 10, port_nr) 0) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(Invalid port number));
+return -1;
+}
+snprintf(port, sizeof(port), %d, port_nr);
+
+/* Get the hostname. */
+n = p - uri; /* n = Length of hostname in bytes. */
+hostname = strdup(uri);
+if (!hostname) {
+virReportOOMError();
+return -1;
+}
+hostname[n] = '\0';
+}
+else {/* hostname (or IP address) */
+hostname = strdup(uri);
+if (!hostname) {
+virReportOOMError();
+return -1;
+}
+}
+*p_hostname = hostname;
+*p_port = atoi(port);
+return 0;
+}
+
+static bool libxlMigrationFromIsAllowed(libxlDriverPrivatePtr driver,
virDomainObjPtr vm)
+{
+/* to be extended */
+VIR_DEBUG(driver=%p, vm=%p, driver, vm);
+return true;
+}
+
+static bool libxlMigrationToIsAllowed(libxlDriverPrivatePtr driver,
virDomainDefPtr def)
+{
+/* to be extended */
+VIR_DEBUG(driver=%p, def=%p, driver, def);
+return true;
+}
I think these functions (and their call sites) can be added in a future
patch that provides an implementation.
+
+static char *
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote: Chun Yan Liu cy...@suse.com 3/6/2012 2:29 PM I didn't get a chance to test this yet, but have some initial review comments. Signed-off-by: Chunyan Liu --- src/libxl/libxl_driver.c | 617 ++ src/libxl/libxl_driver.h | 17 ++- 2 files changed, 632 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d5fa64a..4037635 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -30,6 +30,12 @@ #include #include #include +#include +#include +#include +#include +#include +#include #include internal.h #include logging.h @@ -60,6 +66,13 @@ #define XEN_SCHED_CREDIT_NPARAM 2 static libxlDriverPrivatePtr libxl_driver = NULL; +static virThread migrate_receive_thread; This prevents receiving concurrent migrations. Yes. It is a problem. Defined as static is to be used in Begin3 function virThreadCreate and in Finish3() function virThreadJoin, but actually the thread will exit when receiving data completed or meets error, so virThreadJoin doesn't have much effect here. If a call of virThreadJoin is not needed, then can specify migrate_receive_thread as a local variable. About concurrent migrations, there is another problem in migration port. Every time a migration operation is issued, it will create a socket connection between source and host to transfer data. If a port specified, target side will create a socket and bind to that port, otherwise will bind to a DEFAULT_MIGRATION_PORT (current implementation). But if 1st migration occupies the DEFAULT_MIGRATION_PORT, then 2nd migration (without specifying port) will fail to bind to the DEFAULT_MIGRATION_PORT again. Ah yes, I noticed that but forgot to mention it - sorry. So, to ensure concurrent migrations, perhaps we need a group of ports for migration usage, every migration operation probes for a usable port. How do you think? You can use a virBitmap to keep track of used ports. The qemu driver uses a virBitmap to keep track of used vnc ports, e.g. see qemuProcessNextFreePort() in src/qemu/qemu_process.c. Perhaps the same range of ports qemu uses for migration (i.e. when a port is not specified by the user) can be used in the libxl driver, allowing firewalls and the like to be configured similarly between the two. Thanks, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote:
Hi, Jim,
I made some changes to the patch according to your comments:
a. support concurrent migrations, add virBitmapPtr for probing migration ports
b. update doParseURI:
use virAsprintf instead of strdup and snprintf,
support migration URI syntax hostname[:port], remove xlmigr scheme
c. drop lock of driver while doing doMigrateSend
d. fix extra whitespace and parameter alignment and other places mentioned
Not changed:
a. ensure the name provided in xmlin is the same as def-name. Since we
support
domain name change, so I think the name could be different. Keep not
checked.
b. leaks the original name. It seems the original name won't be used again, so
didn't save original name. Keep it as before.
c. about migrate_receive_args. It's only used in dst side. If send logic and
receive logic still matches, extending the structure won't cause problem.
But if send logic and receive logic cannot match, there will problem. Still
think about how to handle it.
Any further comments will be very appreciated. Thanks for your time!
FYI, your mailer mangled this patch and I had to apply it manually.
Probably best to send with 'git send-email' and include a patch version
(e.g. V2) in the subject. See the archives for examples.
Signed-off-by: Chunyan Liu cy...@suse.com
---
src/libxl/libxl_conf.h |1 +
src/libxl/libxl_driver.c | 634
++
src/libxl/libxl_driver.h | 20 ++-
3 files changed, 653 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 2820afb..dd59817 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -61,6 +61,7 @@ struct _libxlDriverPrivate {
libxl_ctx ctx;
virBitmapPtr reservedVNCPorts;
+virBitmapPtr reservedMigPorts; /* reserved migration ports */
Indentation.
virDomainObjList domains;
virDomainEventStatePtr domainEventState;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d5fa64a..5dc29a0 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -30,6 +30,12 @@
#include math.h
#include libxl.h
#include fcntl.h
+#include stdlib.h
+#include string.h
+#include sys/types.h
+#include sys/socket.h
+#include arpa/inet.h
+#include netdb.h
#include internal.h
#include logging.h
@@ -61,6 +67,12 @@
static libxlDriverPrivatePtr libxl_driver = NULL;
+typedef struct migrate_receive_args {
+virConnectPtr conn;
+virDomainObjPtr vm;
+int sockfd;
+} migrate_receive_args;
+
/* Function declarations */
static int
libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
@@ -810,6 +822,7 @@ libxlShutdown(void)
VIR_FORCE_FCLOSE(libxl_driver-logger_file);
virBitmapFree(libxl_driver-reservedVNCPorts);
+virBitmapFree(libxl_driver-reservedMigPorts);
Indentation.
VIR_FREE(libxl_driver-configDir);
VIR_FREE(libxl_driver-autostartDir);
@@ -865,6 +878,10 @@ libxlStartup(int privileged) {
virBitmapAlloc(LIBXL_VNC_PORT_MAX - LIBXL_VNC_PORT_MIN)) == NULL)
goto out_of_memory;
+if ((libxl_driver-reservedMigPorts =
+ virBitmapAlloc(LIBXL_MIGRATION_MAX_PORT -
LIBXL_MIGRATION_MIN_PORT)) == NULL)
Mailer wrapped this.
+goto out_of_memory;
+
if (virDomainObjListInit(libxl_driver-domains) 0)
goto out_of_memory;
@@ -1095,6 +1112,17 @@ libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED)
return 0;
}
+static int
+libxlSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
+{
+switch (feature) {
+case VIR_DRV_FEATURE_MIGRATION_V3:
+return 1;
+default:
+return 0;
+}
+}
+
static const char *
libxlGetType(virConnectPtr conn ATTRIBUTE_UNUSED)
{
@@ -3842,12 +3870,613 @@ libxlIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED)
return 1;
}
+static int libxlReadFixedMessage(int fd, const void *msg, int msgsz)
Perhaps this should be renamed to libxlCheckMessageBanner() or similar
since you are not only reading a message, but also checking if it
matches the expected banner. E.g.
static int libxlCheckMessageBanner(int fd, const char *banner, int
banner_sz)
+{
+char buf[msgsz];
+
+if (saferead(fd, buf, msgsz) != msgsz || memcmp(buf, msg, msgsz)) {
+return -1;
+}
+
+return 0;
+}
+
+static int doParseURI(const char *uri, char **p_hostname, int *p_port)
+{
+char *p, *hostname;
+int port_nr = 0;
+
+if (uri == NULL)
+return -1;
+
+/* URI passed is a string hostname[:port] */
+if ((p = strrchr(uri, ':')) != NULL) { /* hostname:port */
+int n;
+
+if (virStrToLong_i(p+1, NULL, 10, port_nr) 0) {
+libxlError(VIR_ERR_INVALID_ARG,
+_(Invalid port number));
+return -1;
+}
+
+/* Get the hostname. */
+n = p - uri; /* n =
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote:
2012/3/15 Jim Fehlig jfeh...@suse.com mailto:jfeh...@suse.com
+/* Create socket connection to receive migration data */
+if (!uri_in) {
+hostname = virGetHostname(dconn);
+if (hostname == NULL)
+goto cleanup;
+
+port = libxlFindFreeMigPort(driver,
LIBXL_MIGRATION_MIN_PORT);
I think you should reserve the port in libxlFindFreeMigPort(), similar
to libxlNextFreeVncPort(). In fact, you could probably generalize
libxlNextFreeVncPort(), e.g. libxlNextFreePort(virBitmapPtr
bitmap, int
start_port, int stop_port) and use it to find available VNC and
migration ports.
There is some difference to handle Migration ports and VNC ports:
VNC port always find a free port from VNC ports range and use it, but
migration port could be pointed by user or if not pointed find a free
port to use it. There are two places need to set bitmap maybe:
1. The port pointed by user could be a port in default migration ports
range, we should set bitmap so that next time finding free port could
avoid that port.
If the user is specifying the port, we should just use it and be done.
That is how the VNC port is handled too. If user has specified a vnc
port, then we just use it. Otherwise, call libxlNextFreeVncPort to find
a free one.
2. No port pointed by user, then find a free migration port from
default migration ports range, and set bitmap.
Besides, with port pointed, we need to create socket and bind to the
port too. libxlFindFreeVNCPort creates socket and binds port and set
bitmap in the function, if FindFreeMigPort also does that, then to
user pointed port, we need to do same work again.
libxlFindFreeVNCPort only binds to the port to see if it is in use. If
not in use, it closes the socket, sets the corresponding bit in the
bitmap, and returns the port. Caller then knows the port is free and
available for use, e.g. binding, listening, connecting, or whatever it
pleases to do with the port.
IMO, we could have
static int libxlNextFreePort(virtBitmapPtr bitmap, int startPort, int
numPorts)
which is functionally equivalent to libxlNextFreeVncPort(), but examines
startPort through startPort+numPorts.
Thanks,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Jim Fehlig wrote:
+static int
+libxlDomainMigratePerform3(virDomainPtr dom,
+const char *xmlin ATTRIBUTE_UNUSED,
+const char *cookiein ATTRIBUTE_UNUSED,
+int cookieinlen ATTRIBUTE_UNUSED,
+char **cookieout ATTRIBUTE_UNUSED,
+int *cookieoutlen ATTRIBUTE_UNUSED,
+const char *dconnuri ATTRIBUTE_UNUSED,
+const char *uri,
+unsigned long flags,
+const char *dname ATTRIBUTE_UNUSED,
+unsigned long resource ATTRIBUTE_UNUSED)
+{
+libxlDriverPrivatePtr driver = dom-conn-privateData;
+char *hostname = NULL;
+int port;
+char *servname = NULL;
+struct addrinfo hints, *res;
+int sockfd = -1;
+int ret = -1;
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
+
+libxlDriverLock(driver);
+
+if (doParseURI(uri, hostname, port))
+goto cleanup;
+
+VIR_DEBUG(hostname = %s, port = %d, hostname, port);
+
+if (virAsprintf(servname, %d, port ? port : DEFAULT_MIGRATION_PORT)
0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) 0 ){
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(Failed to create socket));
+goto cleanup;
+}
+
+memset(hints, 0, sizeof(hints));
+hints.ai_family = AF_INET;
+hints.ai_socktype = SOCK_STREAM;
+if (getaddrinfo(hostname, servname, hints, res) || res == NULL) {
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(IP address lookup failed));
+goto cleanup;
+}
+
+if (connect(sockfd, res-ai_addr, res-ai_addrlen) 0) {
+libxlError(VIR_ERR_OPERATION_FAILED,
+ _(Connect error));
+goto cleanup;
+}
+
+ret = doMigrateSend(dom, flags, sockfd);
Hmm, the entire driver is locked during the migration. I just noticed
libxlDomainSave{Flags} also holds the driver lock during save :-(.
libxlDomainCoreDump drops the lock during the dump and IMO migration
should follow the same pattern.
After some more testing, following the pattern in libxlDomainCoreDump is
not a good idea as it leads to a deadlock.
# virsh dump dom /var/lib/libvirt/libxl/save/test.dump
d1. lock driver
d2. get virDomainObjPtr for domain, acquiring dom obj lock
d3. unlock driver
d4. core dump domain
d5. lock driver
d6. do post dump stuff
d7. unlock driver
d8. unlock dom obj lock
While d4 is in progress, list domains
# virsh list
l1. get num domains
l2. lock driver
l3. call virDomainObjListNumOfDomains, which iterates through domains,
obtaining dom obj lock in process
l3 will block when trying to obtain dom obj lock for the domain being
dumped, and note that it is holding the driver lock. When d4 is
finished, it will attempt to acquire the driver lock - deadlock. A
quick fix would be to lock the driver for duration of the dump, similar
to save. But we really need to prevent locking the driver during these
long running operations.
Question for other libvirt devs:
Many of the libxl driver functions use this pattern
- lock driver
- vm = virDomainFindByUUID // acquires dom obj lock
- unlock driver
- do stuff
- virDomainObjUnlock
In some cases, do stuff requires obtaining the driver lock (e.g.
removing a domain from driver's domain list), in which case there is
potential for deadlock. Any suggestions for preventing the deadlock
*and* avoiding locking the driver during long running operations?
Thanks,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Start of freeze for libvirt-0.9.11 and availability of rc1
Daniel Veillard wrote: As scheduled, we are entering the freeze for 0.9.11. I think most API additions are now commited to git upstream (please raise your voice quickly if you see something missing !) I have made a release candidate 1 tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.11-rc1.tar.gz and the git tree is tagged. I think I will make an release candidate 2 tarball on Wednesday, and I'm hoping for a final release on Friday, or maybe Monday if there are issues found. Please give it a try ! Stability and portability feedback are really welcome as we didn't had a release in Feb and the risk of having something messed up is slightly higher than usual ! Some initial observations on a sles11sp2 host Xen 4.1.2 using xl toolstack (xend disabled): # virsh list error: Failed to reconnect to the hypervisor error: no valid connection error: unable to connect to 'localhost:8000': Connection refused qemu/kvm 0.15.1: # virsh capabilities | grep guest # It is late here. I'll take a look at both of these issues tomorrow. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Start of freeze for libvirt-0.9.11 and availability of rc1
Jim Fehlig wrote: Daniel Veillard wrote: As scheduled, we are entering the freeze for 0.9.11. I think most API additions are now commited to git upstream (please raise your voice quickly if you see something missing !) I have made a release candidate 1 tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.11-rc1.tar.gz and the git tree is tagged. I think I will make an release candidate 2 tarball on Wednesday, and I'm hoping for a final release on Friday, or maybe Monday if there are issues found. Please give it a try ! Stability and portability feedback are really welcome as we didn't had a release in Feb and the risk of having something messed up is slightly higher than usual ! Some initial observations on a sles11sp2 host Xen 4.1.2 using xl toolstack (xend disabled): # virsh list error: Failed to reconnect to the hypervisor error: no valid connection error: unable to connect to 'localhost:8000': Connection refused qemu/kvm 0.15.1: # virsh capabilities | grep guest # It is late here. I'll take a look at both of these issues tomorrow. Hmm, strange. I can't reproduce either of these issues today :-/. Looks good now with TCK mostly passing. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-libxl driver defaulting to tap disk and not working (on Fedora 18 and rawhide)
Ian Campbell wrote:
if (l_disk-driverName) {
...
} else {
/* No driverName - default to raw/tap?? */
x_disk-format = LIBXL_DISK_FORMAT_RAW;
x_disk-backend = LIBXL_DISK_BACKEND_TAP;
}
I recall some discussion about the choice of these defaults, but can't
find anything in the xen or libvirt ml archives. We can adjust the
defaults (or defer to libxl?) if it makes sense. I took a quick peek at
newer libxl code (recall this was written against Xen 4.1 libxl) and
perhaps 'x_disk-backend = LIBXL_DISK_BACKEND_QDISK' would be a saner
default? Adding Ian to help with this question...
LIBXL_DISK_BACKEND_UNKNOWN will cause libxl to pick the best available
(which includes checking if blktap is actually there) backend given the
format and file type (block device, file etc). That's probably the right
default.
Ok, thanks.
libxl__device_disk_set_backend is the guy to look at if you are
interested how the selection happens.
After taking a peek, agreed that LIBXL_DISK_BACKEND_UNKNOWN is a better
backend default. I'll leave the format default of LIBXL_DISK_FORMAT_RAW
to be consistent with $xen-src/docs/misc/xl-disk-configuration.txt.
Thanks,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: Fix setting of disk backend
The libxl driver was setting the backend field of libxl_device_disk
structure to LIBXL_DISK_BACKEND_TAP when the driver element of disk
configuration was not specified. This needlessly forces the use of
blktap driver, which may not be loaded in dom0
https://bugzilla.redhat.com/show_bug.cgi?id=912488
Ian Campbell suggested that LIBXL_DISK_BACKEND_UNKNOWN is a better
default in this case
https://www.redhat.com/archives/libvir-list/2013-February/msg01126.html
---
src/libxl/libxl_conf.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 43fb8b1..4ce5dec 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -525,9 +525,13 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk,
libxl_device_disk *x_disk)
return -1;
}
} else {
-/* No driverName - default to raw/tap?? */
+/*
+ * If driverName is not specified, default to raw as per
+ * xl-disk-configuration.txt in the xen documentation and let
+ * libxl pick a suitable backend.
+ */
x_disk-format = LIBXL_DISK_FORMAT_RAW;
-x_disk-backend = LIBXL_DISK_BACKEND_TAP;
+x_disk-backend = LIBXL_DISK_BACKEND_UNKNOWN;
}
/* XXX is this right? */
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Fix setting of disk backend
Eric Blake wrote: On 02/20/2013 01:31 PM, Jim Fehlig wrote: The libxl driver was setting the backend field of libxl_device_disk structure to LIBXL_DISK_BACKEND_TAP when the driver element of disk configuration was not specified. This needlessly forces the use of blktap driver, which may not be loaded in dom0 https://bugzilla.redhat.com/show_bug.cgi?id=912488 Ian Campbell suggested that LIBXL_DISK_BACKEND_UNKNOWN is a better default in this case https://www.redhat.com/archives/libvir-list/2013-February/msg01126.html --- src/libxl/libxl_conf.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) ACK. Thanks, pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix starting qemu instances when apparmor driver is enabled
With the apparmor security driver enabled, qemu instances fail
to start
# grep ^security_driver /etc/libvirt/qemu.conf
security_driver = apparmor
# virsh start test-kvm
error: Failed to start domain test-kvm
error: internal error security label already defined for VM
The model field of virSecurityLabelDef object is always populated
by virDomainDefGetSecurityLabelDef(), so remove the check for a
NULL model when verifying if a label is already defined for the
instance.
Checking for a NULL model and populating it later in
AppArmorGenSecurityLabel() has been left in the code to be
consistent with virSecuritySELinuxGenSecurityLabel().
---
src/security/security_apparmor.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index ddc1fe4..2e6a57f 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -436,8 +436,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return rc;
}
-if ((secdef-label) ||
-(secdef-model) || (secdef-imagelabel)) {
+if (secdef-label || secdef-imagelabel) {
virReportError(VIR_ERR_INTERNAL_ERROR,
%s,
_(security label already defined for VM));
@@ -461,8 +460,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
goto err;
}
-secdef-model = strdup(SECURITY_APPARMOR_NAME);
-if (!secdef-model) {
+if (!secdef-model !(secdef-model = strdup(SECURITY_APPARMOR_NAME))) {
virReportOOMError();
goto err;
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix starting qemu instances when apparmor driver is enabled
Eric Blake wrote: On 02/27/2013 04:51 PM, Jim Fehlig wrote: With the apparmor security driver enabled, qemu instances fail to start # grep ^security_driver /etc/libvirt/qemu.conf security_driver = apparmor # virsh start test-kvm error: Failed to start domain test-kvm error: internal error security label already defined for VM The model field of virSecurityLabelDef object is always populated by virDomainDefGetSecurityLabelDef(), so remove the check for a NULL model when verifying if a label is already defined for the instance. Checking for a NULL model and populating it later in AppArmorGenSecurityLabel() has been left in the code to be consistent with virSecuritySELinuxGenSecurityLabel(). --- src/security/security_apparmor.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) ACK; and safe for 1.0.3. Thanks, pushed now. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix starting qemu instances when apparmor driver is enabled
Guannan Ren wrote: On 03/01/2013 08:37 AM, Jim Fehlig wrote: Eric Blake wrote: On 02/27/2013 04:51 PM, Jim Fehlig wrote: With the apparmor security driver enabled, qemu instances fail to start # grep ^security_driver /etc/libvirt/qemu.conf security_driver = apparmor # virsh start test-kvm error: Failed to start domain test-kvm error: internal error security label already defined for VM The model field of virSecurityLabelDef object is always populated by virDomainDefGetSecurityLabelDef(), so remove the check for a NULL model when verifying if a label is already defined for the instance. Checking for a NULL model and populating it later in AppArmorGenSecurityLabel() has been left in the code to be consistent with virSecuritySELinuxGenSecurityLabel(). --- src/security/security_apparmor.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) ACK; and safe for 1.0.3. Thanks, pushed now. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Hi Jim In selinux, libvirt added a label for tapfd. Do you think this patch makes sense for apparmor? https://www.redhat.com/archives/libvir-list/2012-October/msg01461.html Hi Gunannan, Apologies for missing your initial post of that series. I see that you fixed this exact bug in 2/3 :(. I think 3/3 does make sense for apparmor, but I'm not sure about using AppArmorSetImageFDLabel() as a common function. It returns if secdef-imagelabel == NULL, which would be incorrect if labeling a tap fd right? I promise not to miss the patch if you respin it :). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] libxl_driver: Resolve Coverity errors
Eric Blake wrote: On 03/05/2013 05:43 AM, John Ferlan wrote: 1. The virObjectLock() call was unconditional, but Unlock was conditional on vm being valid. Removed the check 2. A call to virDomainEventNewFromObj() isn't guaranteed to return an event - that check needs to be made prior to libxlDomainEventQueue() of the event. Did not add libxlDriverLock/Unlock around the call since some callers already have lock taken Someday, we should remove the big libxl lock in the same way that danpb just removed the big qemu lock; but that's an independent project. Agreed. I added that to the libxl todo list when I saw his excellent work. Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix starting qemu instances when apparmor driver is enabled
Guannan Ren wrote:
On 03/02/2013 12:41 AM, Jim Fehlig wrote:
Guannan Ren wrote:
Hi Jim
In selinux, libvirt added a label for tapfd.
Do you think this patch makes sense for apparmor?
https://www.redhat.com/archives/libvir-list/2012-October/msg01461.html
Hi Gunannan,
Apologies for missing your initial post of that series. I see that you
fixed this exact bug in 2/3 :(.
I think 3/3 does make sense for apparmor, but I'm not sure about using
AppArmorSetImageFDLabel() as a common function. It returns if
secdef-imagelabel == NULL, which would be incorrect if labeling a tap
fd right?
I promise not to miss the patch if you respin it :).
Regards,
Jim
Nothing to apologize, I really don't know much about apparmor.
The tapfd I mean here
is not used by libvirt deamon, it is a tapfd created on
particular guest which is using macvtap driver
to attach virtual NIC to a given physical interface.
From the code, the secdef-imagelabel have the same value as
secdef-label
which is libvirt-{uuid} file in /etc/apparmor.d/libvirt folder.
If it is null, that means the guest will not
be confined by apparmor, so is this tapfd, I think this is fine.
Yes, agreed.
If you think it is reasonable, I will rebase that patch and send
a v2.
Yep, I think it is reasonable and necessary. I finally got around to
testing your patch and it is indeed needed when using macvtap with
apparmor-confined guests.
Thanks for looking into this!
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] apparmor: use AppArmorSetFDLabel for both imageFD and tapFD
Guannan Ren wrote:
Rename AppArmorSetImageFDLabel to AppArmorSetFDLabel which could
be used as a common function for *ALL* fd relabelling in Linux.
In apparmor profile for specific vm with uuid
cdbebdfa-1d6d-65c3-be0f-fd74b978a773
Path:
/etc/apparmor.d/libvirt/libvirt-cdbebdfa-1d6d-65c3-be0f-fd74b978a773.files
The last line is for the tapfd relabelling.
# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.
/var/log/libvirt/**/rhel6qcow2.log w,
/var/lib/libvirt/**/rhel6qcow2.monitor rw,
/var/run/libvirt/**/rhel6qcow2.pid rwk,
/run/libvirt/**/rhel6qcow2.pid rwk,
/var/run/libvirt/**/*.tunnelmigrate.dest.rhel6qcow2 rw,
/run/libvirt/**/*.tunnelmigrate.dest.rhel6qcow2 rw,
/var/lib/libvirt/images/rhel6u3qcow2.img rw,
/dev/tap45 rw,
---
src/security/security_apparmor.c | 20 +---
1 file changed, 5 insertions(+), 15 deletions(-)
ACK.
diff --git a/src/security/security_apparmor.c
b/src/security/security_apparmor.c
index 2e6a57f..9dd8d74 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -884,9 +884,9 @@ AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr,
}
static int
-AppArmorSetImageFDLabel(virSecurityManagerPtr mgr,
-virDomainDefPtr def,
-int fd)
+AppArmorSetFDLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ int fd)
{
int rc = -1;
char *proc = NULL;
@@ -915,16 +915,6 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr,
return reload_profile(mgr, def, fd_path, true);
}
-/* TODO need code here */
-static int
-AppArmorSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
- virDomainDefPtr def ATTRIBUTE_UNUSED,
- int fd ATTRIBUTE_UNUSED)
-{
-return 0;
-}
-
-
static char *
AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr vm ATTRIBUTE_UNUSED)
@@ -975,8 +965,8 @@ virSecurityDriver virAppArmorSecurityDriver = {
.domainSetSavedStateLabel = AppArmorSetSavedStateLabel,
.domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel,
-.domainSetSecurityImageFDLabel = AppArmorSetImageFDLabel,
-.domainSetSecurityTapFDLabel= AppArmorSetTapFDLabel,
+.domainSetSecurityImageFDLabel = AppArmorSetFDLabel,
+.domainSetSecurityTapFDLabel= AppArmorSetFDLabel,
.domainGetSecurityMountOptions = AppArmorGetMountOptions,
};
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirtd segfault
AL13N wrote: Thread 1 (Thread 0x7fdef683b800 (LWP 20522)): #0 0x in ?? () #1 0x7fdee9a72dc7 in libxl_osevent_occurred_timeout (ctx=optimized out, for_libxl=0x7fdedc001608) at libxl_event.c:1039 #2 0x7fdee9c9ff87 in libxlTimerCallback (timer=optimized out, timer_info=0x7fdedc001730) at libxl/libxl_driver.c:259 #3 0x7fdef5dd0f1a in virEventPollDispatchTimeouts () at util/vireventpoll.c:450 #4 virEventPollRunOnce () at util/vireventpoll.c:643 #5 0x7fdef5dcf88d in virEventRunDefaultImpl () at util/virevent.c:273 #6 0x7fdef5ec96c5 in virNetServerRun (srv=0x1146220) at rpc/virnetserver.c:1108 #7 0x0040c8e0 in main (argc=optimized out, argv=optimized out) at libvirtd.c:1481 I received a similar report on #xendevel just yesterday. I'll take a look at this in the next few days after finishing my current project. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix parsing of bond interface XML
Noticed that parsing bond interface XML containing the miimon element
fails
interface type=bond name=bond0
...
bond mode=active-backup
miimon freq=100 carrier=netif/
...
/bond
/interface
This configuration does not contain the optional updelay and downdelay
attributes, but parsing will fail due to returning the result of
virXPathULong (a -1 when the attribute doesn't exist) from
virInterfaceDefParseBond after examining the updelay attribute.
I considered just adding a ret = 0; near the bottom of
virInterfaceDefParseBond, but see there is no cleanup in the error
label. Instead, just return failure where failure occurs and
return success if the end of the function is reached.
---
src/conf/interface_conf.c | 56 +--
1 file changed, 25 insertions(+), 31 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 9301ec0..3d45d5c 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -572,61 +572,58 @@ error:
static int
virInterfaceDefParseBond(virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
-int ret = -1;
+int res;
unsigned long tmp;
def-data.bond.mode = virInterfaceDefParseBondMode(ctxt);
if (def-data.bond.mode 0)
-goto error;
+return -1;
-ret = virInterfaceDefParseBondItfs(def, ctxt);
-if (ret != 0)
- goto error;
+if (virInterfaceDefParseBondItfs(def, ctxt) != 0)
+return -1;
if (virXPathNode(./miimon[1], ctxt) != NULL) {
def-data.bond.monit = VIR_INTERFACE_BOND_MONIT_MII;
-ret = virXPathULong(string(./miimon/@freq), ctxt, tmp);
-if ((ret == -2) || (ret == -1)) {
+res = virXPathULong(string(./miimon/@freq), ctxt, tmp);
+if ((res == -2) || (res == -1)) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface miimon freq missing or
invalid));
-goto error;
+return -1;
}
def-data.bond.frequency = (int) tmp;
-ret = virXPathULong(string(./miimon/@downdelay), ctxt, tmp);
-if (ret == -2) {
+res = virXPathULong(string(./miimon/@downdelay), ctxt, tmp);
+if (res == -2) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface miimon downdelay invalid));
-goto error;
-} else if (ret == 0) {
+return -1;
+} else if (res == 0) {
def-data.bond.downdelay = (int) tmp;
}
-ret = virXPathULong(string(./miimon/@updelay), ctxt, tmp);
-if (ret == -2) {
+res = virXPathULong(string(./miimon/@updelay), ctxt, tmp);
+if (res == -2) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface miimon updelay invalid));
-goto error;
-} else if (ret == 0) {
+return -1;
+} else if (res == 0) {
def-data.bond.updelay = (int) tmp;
}
def-data.bond.carrier = virInterfaceDefParseBondMiiCarrier(ctxt);
-if (def-data.bond.carrier 0) {
-ret = -1;
-goto error;
-}
+if (def-data.bond.carrier 0)
+return -1;
} else if (virXPathNode(./arpmon[1], ctxt) != NULL) {
def-data.bond.monit = VIR_INTERFACE_BOND_MONIT_ARP;
-ret = virXPathULong(string(./arpmon/@interval), ctxt, tmp);
-if ((ret == -2) || (ret == -1)) {
+res = virXPathULong(string(./arpmon/@interval), ctxt, tmp);
+if ((res == -2) || (res == -1)) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface arpmon interval missing or
invalid));
-goto error;
+return -1;
}
def-data.bond.interval = (int) tmp;
@@ -635,18 +632,15 @@ virInterfaceDefParseBond(virInterfaceDefPtr def,
if (def-data.bond.target == NULL) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface arpmon target missing));
-ret = -1;
-goto error;
+return -1;
}
def-data.bond.validate = virInterfaceDefParseBondArpValid(ctxt);
-if (def-data.bond.validate 0) {
-ret = -1;
-goto error;
-}
+if (def-data.bond.validate 0)
+return -1;
}
-error:
-return ret;
+
+return 0;
}
static int
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2] Fix parsing of bond interface XML
Noticed that parsing bond interface XML containing the miimon element
fails
interface type=bond name=bond0
...
bond mode=active-backup
miimon freq=100 carrier=netif/
...
/bond
/interface
This configuration does not contain the optional updelay and downdelay
attributes, but parsing will fail due to returning the result of
virXPathULong (a -1 when the attribute doesn't exist) from
virInterfaceDefParseBond after examining the updelay attribute.
While fixing this bug, cleanup the function to use virXPathInt instead
of virXPathULong, and store the result directly instead of using a tmp
variable. Using virXPathInt actually fixes a potential silent
truncation bug noted by Eric Blake.
Also, there is no cleaup in the error label. Remove the label,
returning failure where failure occurs and success if the end of the
function is reached.
---
V2:
Use virXPathInt instead of virXPathULong to avoid silent truncation,
and store value directly in the def structure instead of using a tmp
variable.
src/conf/interface_conf.c | 63 ---
1 file changed, 27 insertions(+), 36 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 9301ec0..7ca9c86 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -572,81 +572,72 @@ error:
static int
virInterfaceDefParseBond(virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
-int ret = -1;
-unsigned long tmp;
+int res;
def-data.bond.mode = virInterfaceDefParseBondMode(ctxt);
if (def-data.bond.mode 0)
-goto error;
+return -1;
-ret = virInterfaceDefParseBondItfs(def, ctxt);
-if (ret != 0)
- goto error;
+if (virInterfaceDefParseBondItfs(def, ctxt) != 0)
+return -1;
if (virXPathNode(./miimon[1], ctxt) != NULL) {
def-data.bond.monit = VIR_INTERFACE_BOND_MONIT_MII;
-ret = virXPathULong(string(./miimon/@freq), ctxt, tmp);
-if ((ret == -2) || (ret == -1)) {
+res = virXPathInt(string(./miimon/@freq), ctxt,
+ def-data.bond.frequency);
+if ((res == -2) || (res == -1)) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface miimon freq missing or
invalid));
-goto error;
+return -1;
}
-def-data.bond.frequency = (int) tmp;
-ret = virXPathULong(string(./miimon/@downdelay), ctxt, tmp);
-if (ret == -2) {
+res = virXPathInt(string(./miimon/@downdelay), ctxt,
+ def-data.bond.downdelay);
+if (res == -2) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface miimon downdelay invalid));
-goto error;
-} else if (ret == 0) {
-def-data.bond.downdelay = (int) tmp;
+return -1;
}
-ret = virXPathULong(string(./miimon/@updelay), ctxt, tmp);
-if (ret == -2) {
+res = virXPathInt(string(./miimon/@updelay), ctxt,
+ def-data.bond.updelay);
+if (res == -2) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface miimon updelay invalid));
-goto error;
-} else if (ret == 0) {
-def-data.bond.updelay = (int) tmp;
+return -1;
}
def-data.bond.carrier = virInterfaceDefParseBondMiiCarrier(ctxt);
-if (def-data.bond.carrier 0) {
-ret = -1;
-goto error;
-}
+if (def-data.bond.carrier 0)
+return -1;
} else if (virXPathNode(./arpmon[1], ctxt) != NULL) {
def-data.bond.monit = VIR_INTERFACE_BOND_MONIT_ARP;
-ret = virXPathULong(string(./arpmon/@interval), ctxt, tmp);
-if ((ret == -2) || (ret == -1)) {
+res = virXPathInt(string(./arpmon/@interval), ctxt,
+ def-data.bond.interval);
+if ((res == -2) || (res == -1)) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface arpmon interval missing or
invalid));
-goto error;
+return -1;
}
-def-data.bond.interval = (int) tmp;
def-data.bond.target =
virXPathString(string(./arpmon/@target), ctxt);
if (def-data.bond.target == NULL) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface arpmon target missing));
-ret = -1;
-goto error;
+return -1;
}
def-data.bond.validate = virInterfaceDefParseBondArpValid(ctxt);
-if (def-data.bond.validate 0) {
-ret = -1;
-goto error;
-}
+if (def-data.bond.validate 0)
+return -1;
}
-error:
-return ret;
+
+return 0;
}
static int
--
1.8.0.1
--
libvir-list
Re: [libvirt] [PATCH] Fix parsing of bond interface XML
Eric Blake wrote:
On 03/21/2013 03:50 PM, Jim Fehlig wrote:
Noticed that parsing bond interface XML containing the miimon element
fails
interface type=bond name=bond0
...
bond mode=active-backup
miimon freq=100 carrier=netif/
...
/bond
/interface
This configuration does not contain the optional updelay and downdelay
attributes, but parsing will fail due to returning the result of
virXPathULong (a -1 when the attribute doesn't exist) from
virInterfaceDefParseBond after examining the updelay attribute.
I considered just adding a ret = 0; near the bottom of
virInterfaceDefParseBond, but see there is no cleanup in the error
label. Instead, just return failure where failure occurs and
return success if the end of the function is reached.
---
virInterfaceDefParseBond(virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
-int ret = -1;
+int res;
unsigned long tmp;
Pre-existing, but this is a long...
if (virXPathNode(./miimon[1], ctxt) != NULL) {
def-data.bond.monit = VIR_INTERFACE_BOND_MONIT_MII;
-ret = virXPathULong(string(./miimon/@freq), ctxt, tmp);
-if ((ret == -2) || (ret == -1)) {
+res = virXPathULong(string(./miimon/@freq), ctxt, tmp);
+if ((res == -2) || (res == -1)) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(bond interface miimon freq missing or
invalid));
-goto error;
+return -1;
}
def-data.bond.frequency = (int) tmp;
yet here we want it to be an int. Why not use virXPathInt() on an int
in the first place, so that we don't risk silent truncation? In fact,
why even have tmp, when we can:
ret = virXPathInt(string(./miimon/@freq), ctxt,
def-data.bond.frequency)
in the first place?
The current behavior is to store the value in def depending on the
return value of virXPathULong. But looking at the function, this is
only done when the return value is 0. And it seems the virXPath
functions only store the value when returning 0, so IMO your proposal is
correct. I've sent a V2.
Thanks,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] Fix parsing of bond interface XML
Eric Blake wrote: On 03/21/2013 05:13 PM, Jim Fehlig wrote: Noticed that parsing bond interface XML containing the miimon element fails interface type=bond name=bond0 ... bond mode=active-backup miimon freq=100 carrier=netif/ ... /bond /interface This configuration does not contain the optional updelay and downdelay attributes, but parsing will fail due to returning the result of virXPathULong (a -1 when the attribute doesn't exist) from virInterfaceDefParseBond after examining the updelay attribute. While fixing this bug, cleanup the function to use virXPathInt instead of virXPathULong, and store the result directly instead of using a tmp variable. Using virXPathInt actually fixes a potential silent truncation bug noted by Eric Blake. Also, there is no cleaup in the error label. Remove the label, s/cleaup/cleanup/ Will fix. returning failure where failure occurs and success if the end of the function is reached. --- ACK But I'm going to wait to push this until I test it! After fiddling with the network too much on my test machine, I can no longer reach it. I'll have access to that machine tomorrow and will push this after testing. Thanks, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirtd segfault
AL13N wrote: Op woensdag 20 maart 2013 08:42:52 schreef Jim Fehlig: AL13N wrote: Thread 1 (Thread 0x7fdef683b800 (LWP 20522)): #0 0x in ?? () #1 0x7fdee9a72dc7 in libxl_osevent_occurred_timeout (ctx=optimized out, for_libxl=0x7fdedc001608) at libxl_event.c:1039 #2 0x7fdee9c9ff87 in libxlTimerCallback (timer=optimized out, timer_info=0x7fdedc001730) at libxl/libxl_driver.c:259 #3 0x7fdef5dd0f1a in virEventPollDispatchTimeouts () at util/vireventpoll.c:450 #4 virEventPollRunOnce () at util/vireventpoll.c:643 #5 0x7fdef5dcf88d in virEventRunDefaultImpl () at util/virevent.c:273 #6 0x7fdef5ec96c5 in virNetServerRun (srv=0x1146220) at rpc/virnetserver.c:1108 #7 0x0040c8e0 in main (argc=optimized out, argv=optimized out) at libvirtd.c:1481 I received a similar report on #xendevel just yesterday. I'll take a look at this in the next few days after finishing my current project. Regards, Jim restesting with libvirt-1.0.4 confirmed that it's still unfixed I would have been really surprised if it was fixed :). I did take a few minutes last week to peek at the code, only to realize it is not a simple fix and requires some investigation. Apologies for not having time to do this yet. My schedule for the next several weeks is quite hectic too. I'm adding Bamvor (a SUSE colleague) to the cc list in case he has some free time to investigate as well. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESENT 01/12] libxl: allow script for any network interface, not only bridge
Laine Stump wrote: On 04/10/2013 05:10 AM, Daniel P. Berrange wrote: On Wed, Apr 10, 2013 at 04:44:43AM +0200, Marek Marczykowski wrote: This can be useful for route or NAT networks, or any other custom network setup. Especially configuration example in documentation uses script/ tag with type 'ethernet'. --- src/libxl/libxl_conf.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) The script should not not have been allowed for type='bridge' in the first place, it is only intended for type='ethernet' usage and nothing else. I thought that it was also allowed/necessary for type='bridge' in xen domains. Is this incorrect? I think it is only necessary if something other than the default (/etc/xen/scripts/vif-bridge) is desired. It has been allowed in both xen drivers for as long as I remember. (I agree that it shouldn't be allowed for anything new, though) Agreed, ethernet and bridge only. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESENT 02/12] libxl: PCI passthrough support
Daniel P. Berrange wrote: On Wed, Apr 10, 2013 at 04:44:43AM +0200, Marek Marczykowski wrote: --- src/libxl/libxl_conf.c | 72 ++ src/libxl/libxl_conf.h | 2 ++ 2 files changed, 74 insertions(+) This needs todo more than just create the config. You need to use the libvirt PCI APIs to validate that it is safe to assign the devices requested, and track which devices are given to which guest, to avoid concurrent assignemnt to 2 guests. Basically I'd expect this to borrow all the code currently in the QMEU driver related to the 'activePciHostdevs' and 'inactivePciHostdevs' data structures. Right. Chunyan already posted such a patch series [1], but Laine noted that it would be useful to maintain the state of PCI device assignment for coordination among multiple drivers. Chunyan posted a design [2] for a hostdev passthrough driver, and IIRC she has started coding after receiving feedback from the list. Regards, Jim [1] https://www.redhat.com/archives/libvir-list/2013-January/msg00697.html [2] https://www.redhat.com/archives/libvir-list/2013-March/msg01331.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESENT 04/12] libxl: populate xenstore memory entries at startup
Marek Marczykowski wrote:
libxl uses some xenstore entries for hints in memory management
(especially when starting new domain). This includes dom0 memory limit
and Xen free memory margin, based on current system state. Entries are
created at first usage, so force such usage at daemon startup, which most
likely will be before any domain startup.
Hmm, I'd like to get the xen developer's opinion on this change.
Perhaps Ian C. or Ian J. have some comment...
---
src/libxl/libxl_driver.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 011edf8..89546a5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1115,6 +1115,7 @@ libxlStartup(bool privileged,
char *log_file = NULL;
virCommandPtr cmd;
int status, ret = 0;
+unsigned int free_mem;
uint32_t to match libxl_get_free_memory() definition?
char ebuf[1024];
/* Disable libxl driver if non-root */
@@ -1240,6 +1241,13 @@ libxlStartup(bool privileged,
goto error;
}
+/* This will fill xenstore info about free and dom0 memory - if missing,
+ * should be called before starting first domain */
+if (libxl_get_free_memory(libxl_driver-ctx, free_mem)) {
+VIR_ERROR(_(cannot get free memory info));
+goto error;
+}
Should failure of libxl_get_free_memory() really be fatal and prevent
the driver from loading?
Regards,
Jim
+
if (!(libxl_driver-xmlconf =
virDomainXMLConfNew(libxlDomainXMLPrivateDataCallbacks,
NULL)))
goto error;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] libvirt libxl driver leaking libxl_event's?
On 04/11/2013 07:09 AM, Ian Campbell wrote:
On Thu, 2013-04-11 at 13:46 +0100, Ian Campbell wrote:
Hi Jim,
I don't see any calls to libxl_event_dispose in libvirt.git,
actually I think I meant libxl_event_free, but I can't see that either.
Right. I confirmed that was needed with Ian J. a few weeks (months) back, but
then never submitted a patch. I've attached one, but noticed that the prototype
of the event_occurs callback defines the event parameter as const, causing this
error without the lame cast
cc1: warnings being treated as errors
libxl/libxl_driver.c: In function 'libxlEventHandler':
libxl/libxl_driver.c:760: error: passing argument 2 of 'libxl_event_free'
discards qualifiers from pointer target type
Shouldn't the callback in libxl_event.h just be
void (*event_occurs)(void *user, libxl_event *event);
But I suppose we can't change that. BTW, the const'ness of event led me to
believe it didn't need freed.
Regards,
Jim
libxl_event.h says in relation to the event_occurs callback:
* event becomes owned by the application and must be freed, either
* by event_occurs or later.
so does this mean the memory referenced by event is leaked by
libxlEventHandler?
libxl_event only contains dynamic allocations for
LIBXL_EVENT_TYPE_DISK_EJECT which you don't seem to register for so the
issue is probably benign but worth noting I think.
Ian.
___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9980b38..b9e3f4d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -697,9 +697,10 @@ libxlVmReap(libxlDriverPrivatePtr driver,
* Handle previously registered event notification from libxenlight
*/
static void
-libxlEventHandler(void *data ATTRIBUTE_UNUSED, const libxl_event *event)
+libxlEventHandler(void *data, const libxl_event *event)
{
libxlDriverPrivatePtr driver = libxl_driver;
+libxlDomainObjPrivatePtr priv = ((virDomainObjPtr)data)-privateData;
virDomainObjPtr vm = NULL;
virDomainEventPtr dom_event = NULL;
libxl_shutdown_reason xl_reason = event-u.domain_shutdown.shutdown_reason;
@@ -756,6 +757,7 @@ cleanup:
libxlDomainEventQueue(driver, dom_event);
libxlDriverUnlock(driver);
}
+libxl_event_free(priv-ctx, (libxl_event *)event);
}
static const struct libxl_event_hooks ev_hooks = {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] USB pass-through with XEN
Carlos Rodrigues wrote: Hello everybody, I try to doing USB pass-through with XEN hypervisor using libvirt and i get the following error: # virsh attach-device c6test /tmp/usb_device.xml error: Failed to attach device from /tmp/usb_device.xml error: unsupported configuration: unsupported device type Looking at src/xen/xend_internal.c, I see that only attach/detach of PCI host devices is implemented. There is currently no support for virDomainHostdevSubsysType of VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB. Patches welcome :). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Using leases with virtlockd
Nearly a question for the users list, but I might be encountering a bug too. I'm using virtlockd with 'auto_disk_leases = 0' and struggle with the correct configuration for a lease in the domXML. Using lease lockspacetest-ls/lockspace keytest-lock/key target path='/var/lib/libvirt/lockd'/ /lease results in the following error when starting the domain internal error Lockspace for path /var/lib/libvirt/lockd/test-ls It appears the path attribute + lockspace are used as the Lockspace for this lease. But I'm not sure what Lockspace means in this context. Is it a directory? A file? I assume the latter since it is just flock()ed when the domain is started, right? I'll be happy to provide patches to fix any bugs that might exist here (docs included), but need understand the correct configuration of leases with virtlockd first :). BTW, nice piece of work Daniel! Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] xen: implement virNodeDeviceDetachFlags backend
Laine Stump wrote:
This was the only hypervisor driver other than qemu that implemented
virNodeDeviceDettach. It doesn't currently support multiple pci device
assignment driver backends, but it is simple to plug in this new API,
which will make it easier for Xen people to fill it in later when they
decide to support VFIO (or whatever other) device assignment. Also it
means that management applications will have the same API available to
them for both hypervisors on any given version of libvirt.
The only acceptable value for driverName in this case is NULL, since
there is no alternate, and I'm not willing to pick a name for the
default driver used by Xen.
:-)
ACK to the xen driver changes.
---
src/xen/xen_driver.c | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index a6990cf..8971d4c 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1,7 +1,7 @@
/*
* xen_driver.c: Unified Xen driver.
*
- * Copyright (C) 2007-2012 Red Hat, Inc.
+ * Copyright (C) 2007-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -2133,12 +2133,16 @@ out:
}
static int
-xenUnifiedNodeDeviceDettach(virNodeDevicePtr dev)
+xenUnifiedNodeDeviceDetachFlags(virNodeDevicePtr dev,
+const char *driverName,
+unsigned int flags)
{
virPCIDevicePtr pci;
unsigned domain, bus, slot, function;
int ret = -1;
+virCheckFlags(0, -1);
+
if (xenUnifiedNodeDeviceGetPciInfo(dev, domain, bus, slot, function)
0)
return -1;
@@ -2146,7 +2150,15 @@ xenUnifiedNodeDeviceDettach(virNodeDevicePtr dev)
if (!pci)
return -1;
-if (virPCIDeviceDetach(pci, NULL, NULL, pciback) 0)
+if (!driverName) {
+virPCIDeviceSetStubDriver(pci, pciback);
+} else {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(unknown driver name '%s'), driverName);
+goto out;
+}
+
+if (virPCIDeviceDetach(pci, NULL, NULL, NULL) 0)
goto out;
ret = 0;
@@ -2156,6 +2168,12 @@ out:
}
static int
+xenUnifiedNodeDeviceDettach(virNodeDevicePtr dev)
+{
+return xenUnifiedNodeDeviceDetachFlags(dev, NULL, 0);
+}
+
+static int
xenUnifiedNodeDeviceAssignedDomainId(virNodeDevicePtr dev)
{
int numdomains;
@@ -2405,6 +2423,7 @@ static virDriver xenUnifiedDriver = {
.connectDomainEventRegister = xenUnifiedConnectDomainEventRegister, /*
0.5.0 */
.connectDomainEventDeregister = xenUnifiedConnectDomainEventDeregister,
/* 0.5.0 */
.nodeDeviceDettach = xenUnifiedNodeDeviceDettach, /* 0.6.1 */
+.nodeDeviceDetachFlags = xenUnifiedNodeDeviceDetachFlags, /* 1.0.5 */
.nodeDeviceReAttach = xenUnifiedNodeDeviceReAttach, /* 0.6.1 */
.nodeDeviceReset = xenUnifiedNodeDeviceReset, /* 0.6.1 */
.connectIsEncrypted = xenUnifiedConnectIsEncrypted, /* 0.7.3 */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix segfault during virsh save in pv guest
Bamvor Jian Zhang wrote:
this patch fix the wrong sequence for fd and timeout register. the sequence
was right in dfa1e1dd for fd register, but it changed in e0622ca2.
in this patch, set priv, xl_priv in info and increase info-priv ref count
before virEventAddHandle. if do this after virEventAddHandle, the fd
callback or fd deregister maybe got the empty priv, xl_priv or wrong ref
count.
Bamvor and I discussed this patch off-list while investigating reports
of segfaults in the libxl driver, e.g.
https://www.redhat.com/archives/libvir-list/2013-March/msg00502.html
after apply this patch, test more than 100 rounds passed compare to fail
within 3 rounds without this patch. each round includes define - start -
destroy - create - suspend - resume - reboot - shutdown - save -
resotre - dump - destroy - create - setmem - setvcpus - destroy.
ACK. I'll push this since it is a bug fix.
Regards,
Jim
Signed-off-by: Bamvor Jian Zhang bjzh...@suse.com
---
src/libxl/libxl_driver.c | 39 +--
1 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b4f1889..212d0fc 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -181,26 +181,28 @@ libxlFDRegisterEventHook(void *priv, int fd, void
**hndp,
return -1;
}
+info-priv = priv;
+/*
+ * Take a reference on the domain object. Reference is dropped in
+ * libxlEventHookInfoFree, ensuring the domain object outlives the fd
+ * event objects.
+ */
+virObjectRef(info-priv);
+info-xl_priv = xl_priv;
+
if (events POLLIN)
vir_events |= VIR_EVENT_HANDLE_READABLE;
if (events POLLOUT)
vir_events |= VIR_EVENT_HANDLE_WRITABLE;
+
info-id = virEventAddHandle(fd, vir_events, libxlFDEventCallback,
info, libxlEventHookInfoFree);
if (info-id 0) {
+virObjectUnref(info-priv);
VIR_FREE(info);
return -1;
}
-info-priv = priv;
-/*
- * Take a reference on the domain object. Reference is dropped in
- * libxlEventHookInfoFree, ensuring the domain object outlives the fd
- * event objects.
- */
-virObjectRef(info-priv);
-
-info-xl_priv = xl_priv;
*hndp = info;
return 0;
@@ -283,6 +285,15 @@ libxlTimeoutRegisterEventHook(void *priv,
return -1;
}
+info-priv = priv;
+/*
+ * Also take a reference on the domain object. Reference is dropped in
+ * libxlEventHookInfoFree, ensuring the domain object outlives the
timeout
+ * event objects.
+ */
+virObjectRef(info-priv);
+info-xl_priv = xl_priv;
+
gettimeofday(now, NULL);
timersub(abs_t, now, res);
/* Ensure timeout is not overflowed */
@@ -296,22 +307,14 @@ libxlTimeoutRegisterEventHook(void *priv,
info-id = virEventAddTimeout(timeout, libxlTimerCallback,
info, libxlEventHookInfoFree);
if (info-id 0) {
+virObjectUnref(info-priv);
VIR_FREE(info);
return -1;
}
-info-priv = priv;
-/*
- * Also take a reference on the domain object. Reference is dropped in
- * libxlEventHookInfoFree, ensuring the domain object outlives the
timeout
- * event objects.
- */
-virObjectRef(info-priv);
-
virObjectLock(info-priv);
LIBXL_EV_REG_APPEND(info-priv-timerRegistrations, info);
virObjectUnlock(info-priv);
-info-xl_priv = xl_priv;
*hndp = info;
return 0;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirtd segfault
AL13N wrote: AL13N wrote: Op woensdag 20 maart 2013 08:42:52 schreef Jim Fehlig: AL13N wrote: Thread 1 (Thread 0x7fdef683b800 (LWP 20522)): #0 0x in ?? () #1 0x7fdee9a72dc7 in libxl_osevent_occurred_timeout (ctx=optimized out, for_libxl=0x7fdedc001608) at libxl_event.c:1039 #2 0x7fdee9c9ff87 in libxlTimerCallback (timer=optimized out, timer_info=0x7fdedc001730) at libxl/libxl_driver.c:259 #3 0x7fdef5dd0f1a in virEventPollDispatchTimeouts () at util/vireventpoll.c:450 #4 virEventPollRunOnce () at util/vireventpoll.c:643 #5 0x7fdef5dcf88d in virEventRunDefaultImpl () at util/virevent.c:273 #6 0x7fdef5ec96c5 in virNetServerRun (srv=0x1146220) at rpc/virnetserver.c:1108 #7 0x0040c8e0 in main (argc=optimized out, argv=optimized out) at libvirtd.c:1481 I received a similar report on #xendevel just yesterday. I'll take a look at this in the next few days after finishing my current project. Regards, Jim restesting with libvirt-1.0.4 confirmed that it's still unfixed I would have been really surprised if it was fixed :). /me too FYI, Bamvor fixed a remaining (known) race in the libxl driver, which I've pushed for libvirt 1.0.5 https://www.redhat.com/archives/libvir-list/2013-April/msg01926.html I still want to cleanup handling of domain destruction events as mentioned elsewhere in this thread, but that can wait until support for jobs is added to the driver. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESENT 01/12] libxl: allow script for any network interface, not only bridge
Jim Fehlig wrote: Laine Stump wrote: On 04/10/2013 05:10 AM, Daniel P. Berrange wrote: On Wed, Apr 10, 2013 at 04:44:43AM +0200, Marek Marczykowski wrote: This can be useful for route or NAT networks, or any other custom network setup. Especially configuration example in documentation uses script/ tag with type 'ethernet'. --- src/libxl/libxl_conf.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) The script should not not have been allowed for type='bridge' in the first place, it is only intended for type='ethernet' usage and nothing else. I thought that it was also allowed/necessary for type='bridge' in xen domains. Is this incorrect? I think it is only necessary if something other than the default (/etc/xen/scripts/vif-bridge) is desired. It has been allowed in both xen drivers for as long as I remember. Any consensus here? I double-checked the legacy xen driver and it does in fact support script for interface type='bridge'. I'm not sure how many users specify something other than the default, but I'm loath to break them when moving from the old xen toolstack to libxl. Marek, Assuming we continue to allow script for type='bridge' interfaces in the libxl driver, you'll have to change your patch to only allow it for type 'bridge' and 'ethernet'. BTW, sorry for the delay in getting to your patches. I have a bit of time to work on libxl stuff and will get to reviewing them. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] libvirt, libxl and QDISKs
David Scott wrote:
Something like the attached, which seems to work well for me when
specifying driverName = qemu, e.g.
disk type='file' device='disk'
driver name='qemu'/
source file='/var/lib/xen/images/sles11sp2-pv/disk0.raw'/
target dev='xvda' bus='xen'/
/disk
This also works for me!
Good to hear. I'll send the patch to the libvirt list.
On a related note, what do you think about the attached patch? It
allows the user to select a non-default qemu via the emulator
element. My domain XML looked like this:
devices
emulator/usr/lib/xen/bin/qemu-system-i386/emulator
IMO, the possible emulators should be exposed in the capabilities. E.g.
on a machine with both kvm and qemu, both emulators are shown as
possibilities for an hvm, x86_64 guest
# virsh capabilities
...
guest
os_typehvm/os_type
arch name='x86_64'
wordsize64/wordsize
domain type='qemu'
emulator/usr/bin/qemu-system-x86_64/emulator
machinepc-1.1/machine
machine canonical='pc-1.1'pc/machine
machinepc-1.0/machine
machinepc-0.15/machine
machinepc-0.14/machine
machinepc-0.13/machine
machinepc-0.12/machine
machinepc-0.11/machine
machinepc-0.10/machine
machineisapc/machine
/domain
domain type='kvm'
emulator/usr/bin/qemu-kvm/emulator
machinepc-1.1/machine
machine canonical='pc-1.1'pc/machine
machinepc-1.0/machine
machinepc-0.15/machine
machinepc-0.14/machine
machinepc-0.13/machine
machinepc-0.12/machine
machinepc-0.11/machine
machinepc-0.10/machine
machineisapc/machine
/domain
/guest
...
disk device=disk type=network
driver name='qemu'/
source protocol=rbd name=rbd:rbd/ubuntu1204.img/
target dev=hda/
/disk
graphics type=vnc port=-1 autoport=yes listen=0.0.0.0/
/devices
Now that upstream qemu is the default in xen-4.3, it seems useful to
be able to select the traditional qemu for older VMs.
Agreed. And reporting that both emulators exist via the capabilities
would be helpful for users.
Also I backported this to my xen-4.2 system and used this patch + your
patch + the previous 'stat()' fix to successfully start a VM on ceph
storage via libvirt + libxl (my quest is almost complete!)
Nice!
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f549a5d..abbd3c0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -811,6 +811,30 @@ libxlMakeCapabilities(libxl_ctx *ctx)
}
int
+libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+/* No explicit override means use the default */
+if (!def-emulator) {
+return 0;
+}
+if (strstr(def-emulator, /qemu-system-)) {
+d_config-b_info.device_model_version =
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+return 0;
+}
Here we could check the requested emulator against the capabilities, and
then do the proper mapping for device_model_version.
Do you have time for an upstream libvirt patch to expose the possible
emulators in the capabilities, along with this patch allowing the user
to specify one?
Regards,
Jim
+if (strstr(def-emulator, /qemu-dm)) {
+d_config-b_info.device_model_version =
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+return 0;
+}
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(libxenlight doesn't support emulator '%s'),
+ def-emulator);
+return -1;
+}
+
+
+int
libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainDefPtr def, libxl_domain_config *d_config)
{
@@ -834,6 +858,10 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
goto error;
}
+if (libxlMakeEmulator(def, d_config) 0) {
+goto error;
+}
+
d_config-on_reboot = def-onReboot;
d_config-on_poweroff = def-onPoweroff;
d_config-on_crash = def-onCrash;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] libxl: expose multiple emulators per guest in the capabilities XML
David Scott wrote:
libxl allows users to choose between two standard emulators:
1. (default in xen-4.2): qemu traditional (aka qemu-dm)
2. (default in xen-4.3): qemu upstream (aka qemu-system-i386)
The person who builds and packages xen gets to choose which
emulators are built. We examine the filesystem for the emulators
at runtime and expose them as separate domains within the same
guest architecture.
Signed-off-by: David Scott dave.sc...@eu.citrix.com
---
src/libxl/libxl_conf.c | 87 ---
1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 7e0753a..472d116 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -29,6 +29,8 @@
#include libxl.h
#include sys/types.h
#include sys/socket.h
+#include sys/stat.h
+#include unistd.h
#include internal.h
#include virlog.h
@@ -50,6 +52,28 @@
/* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */
#define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type {
+emulator_traditional = 0,
+emulator_upstream= 1,
+emulator_last= 2,
+/* extend with specific qemu versions later */
+};
Do you think this will need to be extended in the future? As
'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+
+#define EMULATOR_LIB64 /usr/lib64/xen/bin/
+#define EMULATOR_LIB32 /usr/lib/xen/bin/
+
+#define EMULATOR_TRADITIONAL qemu-dm
+#define EMULATOR_UPSTREAMqemu-system-i386
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR /xen/bin/qemu-dm
#define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR /xen/bin/qemu-sytstem-i386
+
+static const char* emulator_lib64_path [] = {
+EMULATOR_LIB64 EMULATOR_TRADITIONAL,
+EMULATOR_LIB64 EMULATOR_UPSTREAM,
+};
+
+static const char* emulator_lib32_path [] = {
+EMULATOR_LIB32 EMULATOR_TRADITIONAL,
+EMULATOR_LIB32 EMULATOR_UPSTREAM,
+};
struct guest_arch {
virArch arch;
@@ -68,10 +92,11 @@ static virCapsPtr
libxlBuildCapabilities(virArch hostarch,
int host_pae,
struct guest_arch *guest_archs,
- int nr_guest_archs)
+ int nr_guest_archs,
+ int emulators_found[])
{
virCapsPtr caps;
-int i;
+int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL)
goto no_memory;
@@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch,
if ((guest = virCapabilitiesAddGuest(caps,
guest_archs[i].hvm ? hvm :
xen,
guest_archs[i].arch,
- ((hostarch == VIR_ARCH_X86_64) ?
- /usr/lib64/xen/bin/qemu-dm :
- /usr/lib/xen/bin/qemu-dm),
- (guest_archs[i].hvm ?
- /usr/lib/xen/boot/hvmloader :
- NULL),
+ NULL,
+ NULL,
1,
machines)) == NULL) {
virCapabilitiesFreeMachines(machines, 1);
@@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch,
}
machines = NULL;
-if (virCapabilitiesAddGuestDomain(guest,
- xen,
- NULL,
- NULL,
- 0,
- NULL) == NULL)
-goto no_memory;
+for (j = 0; j emulator_last; ++j) {
+if (emulators_found[j] == -1) /* failure from stat(2) */
+continue;
+if (virCapabilitiesAddGuestDomain(guest,
+ xen,
+ ((hostarch == VIR_ARCH_X86_64)
?
+ emulator_lib64_path[j] :
+ emulator_lib32_path[j]),
+ (guest_archs[i].hvm ?
+ /usr/lib/xen/boot/hvmloader
:
+ NULL),
+ 0,
+ NULL) == NULL)
+goto no_memory;
+}
and then just add the emulators here. E.g.
if (virFileExists(LIBXL_EMULATOR_TRADITIONAL_PATH) {
if (virCapabilitiesAddGuestDomain(guest,
Re: [libvirt] [PATCH 2/2] libxl: allow an emulator to be selected in the domain config XML
David Scott wrote:
We cross-check the given path against the capabilties, and translate
it into a libxl_device_model_version.
Signed-off-by: David Scott dave.sc...@eu.citrix.com
---
src/libxl/libxl_conf.c | 41 +
1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 472d116..868d0cf 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -75,6 +75,11 @@ static const char* emulator_lib32_path [] = {
EMULATOR_LIB32 EMULATOR_UPSTREAM,
};
+static const libxl_device_model_version emulator_to_device_model [] = {
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL,
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN,
+};
+
struct guest_arch {
virArch arch;
int bits;
@@ -833,6 +838,38 @@ libxlMakeCapabilities(libxl_ctx *ctx)
}
int
+libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+virArch hostarch;
+const char *path;
+int i;
+
+/* No explicit override means use the default */
+if (!def-emulator) {
+return 0;
+}
+
+hostarch = virArchFromHost();
+
+for (i = 0; i emulator_last; ++i) {
+ path = ((hostarch == VIR_ARCH_X86_64) ?
+ emulator_lib64_path[i] :
+ emulator_lib32_path[i]);
+ if (STREQ(path, def-emulator)) {
I thought there was a virCapabilitiesSupportsGuestEmulator() or similar,
but I don't see it in src/conf/capabilities.c. I think it makes sense
to add such a function to the capabilities and then just call it here,
passing the requested emulator. Perhaps other libvirt developers can
comment on the usefulness of virCapabilitiesSupportGuestEmulator().
+ d_config-b_info.device_model_version =
+ emulator_to_device_model[i];
+ return 0;
+ }
+}
+
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(libxenlight doesn't support emulator '%s'),
+ def-emulator);
+return -1;
+}
+
+
+int
libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainDefPtr def, libxl_domain_config *d_config)
{
@@ -856,6 +893,10 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
goto error;
}
+if (libxlMakeEmulator(def, d_config) 0) {
The capabilities created when the libxl driver is loaded are available
in libxlDriverPrivatePtr and could be passed to libxlMakeEmulator()
libxlMakeEmulator(driver-caps, def, d_config)
Regards,
Jim
+goto error;
+}
+
d_config-on_reboot = def-onReboot;
d_config-on_poweroff = def-onPoweroff;
d_config-on_crash = def-onCrash;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] libxl: expose multiple emulators per guest in the capabilities XML
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators:
1. (default in xen-4.2): qemu traditional (aka qemu-dm)
2. (default in xen-4.3): qemu upstream (aka qemu-system-i386)
The person who builds and packages xen gets to choose which
emulators are built. We examine the filesystem for the emulators
at runtime and expose them as separate domains within the same
guest architecture.
Signed-off-by: David Scott dave.sc...@eu.citrix.com
---
src/libxl/libxl_conf.c | 87
---
1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 7e0753a..472d116 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -29,6 +29,8 @@
#include libxl.h
#include sys/types.h
#include sys/socket.h
+#include sys/stat.h
+#include unistd.h
#include internal.h
#include virlog.h
@@ -50,6 +52,28 @@
/* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */
#define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type {
+emulator_traditional = 0,
+emulator_upstream= 1,
+emulator_last= 2,
+/* extend with specific qemu versions later */
+};
Do you think this will need to be extended in the future? As
'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+
+#define EMULATOR_LIB64 /usr/lib64/xen/bin/
+#define EMULATOR_LIB32 /usr/lib/xen/bin/
+
+#define EMULATOR_TRADITIONAL qemu-dm
+#define EMULATOR_UPSTREAMqemu-system-i386
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR /xen/bin/qemu-dm
#define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR /xen/bin/qemu-sytstem-i386
+
+static const char* emulator_lib64_path [] = {
+EMULATOR_LIB64 EMULATOR_TRADITIONAL,
+EMULATOR_LIB64 EMULATOR_UPSTREAM,
+};
+
+static const char* emulator_lib32_path [] = {
+EMULATOR_LIB32 EMULATOR_TRADITIONAL,
+EMULATOR_LIB32 EMULATOR_UPSTREAM,
+};
struct guest_arch {
virArch arch;
@@ -68,10 +92,11 @@ static virCapsPtr
libxlBuildCapabilities(virArch hostarch,
int host_pae,
struct guest_arch *guest_archs,
- int nr_guest_archs)
+ int nr_guest_archs,
+ int emulators_found[])
{
virCapsPtr caps;
-int i;
+int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL)
goto no_memory;
@@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch,
if ((guest = virCapabilitiesAddGuest(caps,
guest_archs[i].hvm ? hvm :
xen,
guest_archs[i].arch,
- ((hostarch ==
VIR_ARCH_X86_64) ?
- /usr/lib64/xen/bin/qemu-dm
:
- /usr/lib/xen/bin/qemu-dm),
- (guest_archs[i].hvm ?
-
/usr/lib/xen/boot/hvmloader :
- NULL),
+ NULL,
+ NULL,
1,
machines)) == NULL) {
virCapabilitiesFreeMachines(machines, 1);
@@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch,
}
machines = NULL;
-if (virCapabilitiesAddGuestDomain(guest,
- xen,
- NULL,
- NULL,
- 0,
- NULL) == NULL)
-goto no_memory;
+for (j = 0; j emulator_last; ++j) {
+if (emulators_found[j] == -1) /* failure from stat(2) */
+continue;
+if (virCapabilitiesAddGuestDomain(guest,
+ xen,
+ ((hostarch ==
VIR_ARCH_X86_64) ?
+ emulator_lib64_path[j] :
+ emulator_lib32_path[j]),
+ (guest_archs[i].hvm ?
+
/usr/lib/xen/boot/hvmloader :
+ NULL),
+ 0,
+ NULL) == NULL)
+goto no_memory;
+}
and then just add
Re: [libvirt] [PATCH 2/2] libxl: allow an emulator to be selected in the domain config XML
Jim Fehlig wrote:
David Scott wrote:
We cross-check the given path against the capabilties, and translate
it into a libxl_device_model_version.
Signed-off-by: David Scott dave.sc...@eu.citrix.com
---
src/libxl/libxl_conf.c | 41 +
1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 472d116..868d0cf 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -75,6 +75,11 @@ static const char* emulator_lib32_path [] = {
EMULATOR_LIB32 EMULATOR_UPSTREAM,
};
+static const libxl_device_model_version emulator_to_device_model [] = {
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL,
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN,
+};
+
struct guest_arch {
virArch arch;
int bits;
@@ -833,6 +838,38 @@ libxlMakeCapabilities(libxl_ctx *ctx)
}
int
+libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+virArch hostarch;
+const char *path;
+int i;
+
+/* No explicit override means use the default */
+if (!def-emulator) {
+return 0;
+}
+
+hostarch = virArchFromHost();
+
+for (i = 0; i emulator_last; ++i) {
+ path = ((hostarch == VIR_ARCH_X86_64) ?
+ emulator_lib64_path[i] :
+ emulator_lib32_path[i]);
+ if (STREQ(path, def-emulator)) {
I thought there was a virCapabilitiesSupportsGuestEmulator() or similar,
but I don't see it in src/conf/capabilities.c. I think it makes sense
to add such a function to the capabilities and then just call it here,
passing the requested emulator. Perhaps other libvirt developers can
comment on the usefulness of virCapabilitiesSupportGuestEmulator().
Daniel already clarified this, so no need to do even more work David :).
As mentioned in my other response [1], your original patch (with a check
to verify the requested emulator exists) should be sufficient.
Regards,
Jim
[1] https://www.redhat.com/archives/libvir-list/2013-April/msg02087.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: Fix double-dispose of libxl domain config
libxlBuildDomainConfig() was disposing the libxl_domain_config object
on error, only to have it disposed again by libxlBuildDomainConfig()'s
caller, which resulted in a segfault. Leave disposing of the config
object to it's owner.
---
src/libxl/libxl_conf.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 7e0753a..3de642b 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -796,19 +796,19 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
return -1;
if (libxlMakeDomBuildInfo(def, d_config) 0) {
-goto error;
+return -1;
}
if (libxlMakeDiskList(def, d_config) 0) {
-goto error;
+return -1;
}
if (libxlMakeNicList(def, d_config) 0) {
-goto error;
+return -1;
}
if (libxlMakeVfbList(driver, def, d_config) 0) {
-goto error;
+return -1;
}
d_config-on_reboot = def-onReboot;
@@ -816,8 +816,4 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
d_config-on_crash = def-onCrash;
return 0;
-
-error:
-libxl_domain_config_dispose(d_config);
-return -1;
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] libxl: Add support for qdisk disk backend
This small patch series adds support for qdisk backend type in libxl. A qdisk uses the block drivers in qemu to serve as a block backend, verses blktap or blkbk. While testing the second patch, I noticed a slightly misleading error was emitted when the tap backend didn't support the requested disk format. Fix this with the first patch, including adding format checks in the other supported disk backends. Jim Fehlig (2): libxl: Fix disk format error message libxl: support qdisk backend src/libxl/libxl_conf.c | 49 +++-- 1 file changed, 47 insertions(+), 2 deletions(-) -- 1.8.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] libxl: support qdisk backend
libxl supports the LIBXL_DISK_BACKEND_QDISK disk backend, where qemu
is used to provide the disk backend. This patch simply maps the
existing driver name='qemu'/ to LIBXL_DISK_BACKEND_QDISK.
---
src/libxl/libxl_conf.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e646ec5..1ddb1f5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -501,6 +501,31 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk,
libxl_device_disk *x_disk)
l_disk-driverName);
return -1;
}
+} else if (STREQ(l_disk-driverName, qemu)) {
+x_disk-backend = LIBXL_DISK_BACKEND_QDISK;
+switch (l_disk-format) {
+case VIR_STORAGE_FILE_QCOW:
+x_disk-format = LIBXL_DISK_FORMAT_QCOW;
+break;
+case VIR_STORAGE_FILE_QCOW2:
+x_disk-format = LIBXL_DISK_FORMAT_QCOW2;
+break;
+case VIR_STORAGE_FILE_VHD:
+x_disk-format = LIBXL_DISK_FORMAT_VHD;
+break;
+case VIR_STORAGE_FILE_NONE:
+/* No subtype specified, default to raw */
+case VIR_STORAGE_FILE_RAW:
+x_disk-format = LIBXL_DISK_FORMAT_RAW;
+break;
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(libxenlight does not support disk format %s
+ with disk driver %s),
+
virStorageFileFormatTypeToString(l_disk-format),
+ l_disk-driverName);
+return -1;
+}
} else if (STREQ(l_disk-driverName, file)) {
if (l_disk-format != VIR_STORAGE_FILE_NONE ||
l_disk-format != VIR_STORAGE_FILE_RAW) {
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] libxl: Fix disk format error message
Specifying an unsupported disk format with the tap driver resulted in
a less than helpful error message
error: Failed to start domain test-hvm
error: internal error libxenlight does not support disk driver qed
Change the message to state that the qed format is not supported by
the tap driver, e.g.
error: Failed to start domain test-hvm
error: internal error libxenlight does not support disk format qed
with disk driver tap
While at it, check for unsupported formats in the other driver
backends.
---
src/libxl/libxl_conf.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 7e0753a..e646ec5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -495,14 +495,34 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk,
libxl_device_disk *x_disk)
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(libxenlight does not support disk driver
%s),
-
virStorageFileFormatTypeToString(l_disk-format));
+ _(libxenlight does not support disk format %s
+ with disk driver %s),
+
virStorageFileFormatTypeToString(l_disk-format),
+ l_disk-driverName);
return -1;
}
} else if (STREQ(l_disk-driverName, file)) {
+if (l_disk-format != VIR_STORAGE_FILE_NONE ||
+l_disk-format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(libxenlight does not support disk format %s
+ with disk driver %s),
+
virStorageFileFormatTypeToString(l_disk-format),
+ l_disk-driverName);
+return -1;
+}
x_disk-format = LIBXL_DISK_FORMAT_RAW;
x_disk-backend = LIBXL_DISK_BACKEND_TAP;
} else if (STREQ(l_disk-driverName, phy)) {
+if (l_disk-format != VIR_STORAGE_FILE_NONE ||
+l_disk-format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(libxenlight does not support disk format %s
+ with disk driver %s),
+
virStorageFileFormatTypeToString(l_disk-format),
+ l_disk-driverName);
+return -1;
+}
x_disk-format = LIBXL_DISK_FORMAT_RAW;
x_disk-backend = LIBXL_DISK_BACKEND_PHY;
} else {
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] libxl: expose multiple emulators per guest in the capabilities XML
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators:
1. (default in xen-4.2): qemu traditional (aka qemu-dm)
2. (default in xen-4.3): qemu upstream (aka qemu-system-i386)
The person who builds and packages xen gets to choose which
emulators are built. We examine the filesystem for the emulators
at runtime and expose them as separate domains within the same
guest architecture.
Signed-off-by: David Scott dave.sc...@eu.citrix.com
---
src/libxl/libxl_conf.c | 87
---
1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 7e0753a..472d116 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -29,6 +29,8 @@
#include libxl.h
#include sys/types.h
#include sys/socket.h
+#include sys/stat.h
+#include unistd.h
#include internal.h
#include virlog.h
@@ -50,6 +52,28 @@
/* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */
#define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type {
+emulator_traditional = 0,
+emulator_upstream= 1,
+emulator_last= 2,
+/* extend with specific qemu versions later */
+};
Do you think this will need to be extended in the future? As
'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+
+#define EMULATOR_LIB64 /usr/lib64/xen/bin/
+#define EMULATOR_LIB32 /usr/lib/xen/bin/
+
+#define EMULATOR_TRADITIONAL qemu-dm
+#define EMULATOR_UPSTREAMqemu-system-i386
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR /xen/bin/qemu-dm
#define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR /xen/bin/qemu-sytstem-i386
+
+static const char* emulator_lib64_path [] = {
+EMULATOR_LIB64 EMULATOR_TRADITIONAL,
+EMULATOR_LIB64 EMULATOR_UPSTREAM,
+};
+
+static const char* emulator_lib32_path [] = {
+EMULATOR_LIB32 EMULATOR_TRADITIONAL,
+EMULATOR_LIB32 EMULATOR_UPSTREAM,
+};
struct guest_arch {
virArch arch;
@@ -68,10 +92,11 @@ static virCapsPtr
libxlBuildCapabilities(virArch hostarch,
int host_pae,
struct guest_arch *guest_archs,
- int nr_guest_archs)
+ int nr_guest_archs,
+ int emulators_found[])
{
virCapsPtr caps;
-int i;
+int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL)
goto no_memory;
@@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch,
if ((guest = virCapabilitiesAddGuest(caps,
guest_archs[i].hvm ? hvm :
xen,
guest_archs[i].arch,
- ((hostarch ==
VIR_ARCH_X86_64) ?
- /usr/lib64/xen/bin/qemu-dm
:
- /usr/lib/xen/bin/qemu-dm),
- (guest_archs[i].hvm ?
-
/usr/lib/xen/boot/hvmloader :
- NULL),
+ NULL,
+ NULL,
1,
machines)) == NULL) {
virCapabilitiesFreeMachines(machines, 1);
@@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch,
}
machines = NULL;
-if (virCapabilitiesAddGuestDomain(guest,
- xen,
- NULL,
- NULL,
- 0,
- NULL) == NULL)
-goto no_memory;
+for (j = 0; j emulator_last; ++j) {
+if (emulators_found[j] == -1) /* failure from stat(2) */
+continue;
+if (virCapabilitiesAddGuestDomain(guest,
+ xen,
+ ((hostarch ==
VIR_ARCH_X86_64) ?
+ emulator_lib64_path[j] :
+ emulator_lib32_path[j]),
+ (guest_archs[i].hvm ?
+
/usr/lib/xen/boot/hvmloader :
+ NULL),
+ 0,
+ NULL) == NULL)
+goto no_memory;
+}
and then just add
Re: [libvirt] [PATCH 1/2] libxl: expose multiple emulators per guest in the capabilities XML
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 11:01:04PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators:
1. (default in xen-4.2): qemu traditional (aka qemu-dm)
2. (default in xen-4.3): qemu upstream (aka qemu-system-i386)
The person who builds and packages xen gets to choose which
emulators are built. We examine the filesystem for the emulators
at runtime and expose them as separate domains within the same
guest architecture.
Signed-off-by: David Scott dave.sc...@eu.citrix.com
---
src/libxl/libxl_conf.c | 87
---
1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 7e0753a..472d116 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -29,6 +29,8 @@
#include libxl.h
#include sys/types.h
#include sys/socket.h
+#include sys/stat.h
+#include unistd.h
#include internal.h
#include virlog.h
@@ -50,6 +52,28 @@
/* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */
#define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type {
+emulator_traditional = 0,
+emulator_upstream= 1,
+emulator_last= 2,
+/* extend with specific qemu versions later */
+};
Do you think this will need to be extended in the future? As
'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+
+#define EMULATOR_LIB64 /usr/lib64/xen/bin/
+#define EMULATOR_LIB32 /usr/lib/xen/bin/
+
+#define EMULATOR_TRADITIONAL qemu-dm
+#define EMULATOR_UPSTREAMqemu-system-i386
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR /xen/bin/qemu-dm
#define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR /xen/bin/qemu-sytstem-i386
+
+static const char* emulator_lib64_path [] = {
+EMULATOR_LIB64 EMULATOR_TRADITIONAL,
+EMULATOR_LIB64 EMULATOR_UPSTREAM,
+};
+
+static const char* emulator_lib32_path [] = {
+EMULATOR_LIB32 EMULATOR_TRADITIONAL,
+EMULATOR_LIB32 EMULATOR_UPSTREAM,
+};
struct guest_arch {
virArch arch;
@@ -68,10 +92,11 @@ static virCapsPtr
libxlBuildCapabilities(virArch hostarch,
int host_pae,
struct guest_arch *guest_archs,
- int nr_guest_archs)
+ int nr_guest_archs,
+ int emulators_found[])
{
virCapsPtr caps;
-int i;
+int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL)
goto no_memory;
@@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch,
if ((guest = virCapabilitiesAddGuest(caps,
guest_archs[i].hvm ? hvm
: xen,
guest_archs[i].arch,
- ((hostarch ==
VIR_ARCH_X86_64) ?
-
/usr/lib64/xen/bin/qemu-dm :
-
/usr/lib/xen/bin/qemu-dm),
- (guest_archs[i].hvm ?
-
/usr/lib/xen/boot/hvmloader :
- NULL),
+ NULL,
+ NULL,
1,
machines)) == NULL) {
virCapabilitiesFreeMachines(machines, 1);
@@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch,
}
machines = NULL;
-if (virCapabilitiesAddGuestDomain(guest,
- xen,
- NULL,
- NULL,
- 0,
- NULL) == NULL)
-goto no_memory;
+for (j = 0; j emulator_last; ++j) {
+if (emulators_found[j] == -1) /* failure from stat(2) */
+continue;
+if (virCapabilitiesAddGuestDomain(guest,
+ xen,
+ ((hostarch ==
VIR_ARCH_X86_64) ?
+ emulator_lib64_path[j] :
+ emulator_lib32_path[j]),
+ (guest_archs[i].hvm ?
+
/usr/lib/xen/boot/hvmloader :
+ NULL
Re: [libvirt] [PATCH] libxl: Fix double-dispose of libxl domain config
Eric Blake wrote: On 04/29/2013 05:20 PM, Jim Fehlig wrote: libxlBuildDomainConfig() was disposing the libxl_domain_config object on error, only to have it disposed again by libxlBuildDomainConfig()'s caller, which resulted in a segfault. Leave disposing of the config object to it's owner. --- src/libxl/libxl_conf.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) ACK. Thanks, pushed for 1.0.5 since it's strictly a bug fix. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: allow an emulator to be selected in the domain config XML
David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the
device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning upstream qemu, the default in xen-4.3 onwards
...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename:
if it is qemu-dm then it's the old xen-specific fork. If anything
else then we assume upstream qemu (whose filename may change
in future). Note that if you are using a wrapper script to (eg)
adjust the arguments of the old qemu during development, you will
have to ensure the wrapper script also has the name qemu-dm, by
placing it in a separate directory.
That is unfortunate. Users could have existing config with
emulator/usr/bin/my-qemu-dm/emulator which works with the legacy
stack but not with libxl right? Is it possible to safely query the
binary to determine if it is qemu-dm?
Regards,
Jim
Signed-off-by: David Scott dave.sc...@eu.citrix.com
---
src/libxl/libxl_conf.c | 44
1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 7e0753a..2aa5a62 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -788,6 +788,46 @@ libxlMakeCapabilities(libxl_ctx *ctx)
}
int
+libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+/* No explicit override means use the default */
+if (!def-emulator) {
+return 0;
+}
+
+if (!virFileExists(def-emulator)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(emulator '%s' not found),
+ def-emulator);
+return -1;
+}
+
+VIR_FREE(d_config-b_info.device_model);
+if ((d_config-b_info.device_model = strdup(def-emulator)) == NULL) {
+virReportOOMError();
+return -1;
+}
+
+/* N.B. from xen/tools/libxl/libxl_types.idl:
+ * If setting device_model you must set device_model_version too.
+ *
+ * The xen-4.3 and later default is upstream qemu (QEMU_XEN)
+ * so we make that the default and special-case the old-style
+ * traditional qemu (QEMU_XEN_TRADITIONAL)
+ */
+
+d_config-b_info.device_model_version =
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+
+if (STREQ(basename(def-emulator), qemu-dm))
+d_config-b_info.device_model_version =
+LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+
+return 0;
+}
+
+
+int
libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainDefPtr def, libxl_domain_config *d_config)
{
@@ -811,6 +851,10 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
goto error;
}
+if (libxlMakeEmulator(def, d_config) 0) {
+goto error;
+}
+
d_config-on_reboot = def-onReboot;
d_config-on_poweroff = def-onPoweroff;
d_config-on_crash = def-onCrash;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: allow an emulator to be selected in the domain config XML
David Scott wrote: Hi, [added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.] On 30/04/13 16:10, Jim Fehlig wrote: David Scott wrote: The emulator path supplied can be any valid path on the system. Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either ...QEMU_XEN: meaning upstream qemu, the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork We detect the device_model_version by examining the qemu filename: if it is qemu-dm then it's the old xen-specific fork. If anything else then we assume upstream qemu (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name qemu-dm, by placing it in a separate directory. That is unfortunate. Users could have existing config with emulator/usr/bin/my-qemu-dm/emulator which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm? From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think? I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it. The other options I can think of are: 1. weaken the test so we interpret any filename containing the substring qemu-dm as traditional-- this would catch your case at least Right, it would probably catch a lot of cases. But users are free to have names with no 'qemu-dm' component. 2. flip the default around so that if an emulator is provided we assume traditional unless the filename is qemu-system-i386 (or maybe just contains qemu-system-i386 or contains qemu-system) How is this handled in xl? There is certainly a lot of xm config out there with device_model=/usr/lib/xen/bin/qemu-dm Are users expected to add device_model_version=qemu-traditional when migrating to xl/libxl? I suppose xl handles this (should just look at the code), but it seems an implementation detail that shouldn't be exposed to 3rd party apps. 3. add libxl driver-specific XML (is that possible?) to allow the user to override a libvirt default. It would be a shame to expose the complexity to the libvirt client though. There is such functionality for qemu, but I would prefer to avoid it for this case, particularly since it works in the legacy xen driver. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: allow an emulator to be selected in the domain config XML
Ian Campbell wrote: On Wed, 2013-05-01 at 04:42 +0100, Jim Fehlig wrote: David Scott wrote: Hi, [added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.] On 30/04/13 16:10, Jim Fehlig wrote: David Scott wrote: The emulator path supplied can be any valid path on the system. Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either ...QEMU_XEN: meaning upstream qemu, the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork We detect the device_model_version by examining the qemu filename: if it is qemu-dm then it's the old xen-specific fork. If anything else then we assume upstream qemu (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name qemu-dm, by placing it in a separate directory. That is unfortunate. Users could have existing config with emulator/usr/bin/my-qemu-dm/emulator which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm? From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think? I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it. The intended usage model is the other way around, we expect normal users to just ask for a specific device model version and for them not to care about the specific path to the device model binary. That doesn't seem right to me. In a few years time who will care about qemu-traditional at all? Seems more useful for me to be able to say use '/usr/bin/qemu-system-i386', '/usr/bin/qemu-system-arm', '/usr/lib/xen/bin/qemu-dm', '/usr/local/bin/qemu-add-my-args', etc. And if I don't specify an emulator, then pick a sane default. Most users won't even do that and will just want to accept the default device model version. Only users with preexisting VMs which are reliant on the older device model for some reason (e.g. Windows Genuine Advantage or whatever it is called) or, historically, people who wanted to try out the new device model before it became the default would normally want to specify the device model version at all. Only advanced users should want to override the actual path to use a specific device model binary. The _override suffix on device_model_override is intended to imply advanced use, you better know what you are doing, and that includes knowing which version it is (since there is no obvious way to detect, other than the sorts of heuristics you are discussing here). This is probably best documented in the xl.cfg(5) manpage rather than in anything libxl-ish (sorry): =item Bdevice_model_version=DEVICE-MODEL Selects which variant of the device-model should be used for this guest. Valid values are: ... the obvious list ... =back It is recommended to accept the default value for new guests. If you have existing guests then, depending on the nature of the guest Operating System, you may wish to force them to use the device model which they were installed with. ... =item Bdevice_model_override=PATH Override the path to the binary to be used as the device-model. The binary provided here MUST be consistent with the `device_model_version` which you have specified. You should not normally need to specify this option. The other options I can think of are: 1. weaken the test so we interpret any filename containing the substring qemu-dm as traditional-- this would catch your case at least Right, it would probably catch a lot of cases. But users are free to have names with no 'qemu-dm' component. 2. flip the default around so that if an emulator is provided we assume traditional unless the filename is qemu-system-i386 (or maybe just contains qemu-system-i386 or contains qemu-system) How is this handled in xl? There is certainly a lot of xm config out there with device_model=/usr/lib/xen/bin/qemu-dm xl will ignore this and print a warning, fprintf(stderr, WARNING: ignoring device_model directive.\n WARNING: Use \device_model_override\ instead if you really want a non-default
Re: [libvirt] [Xen-devel] [PATCH] libxl: allow an emulator to be selected in the domain config XML
Ian Campbell wrote: On Wed, 2013-05-01 at 15:31 +0100, Jim Fehlig wrote: Ian Campbell wrote: On Wed, 2013-05-01 at 04:42 +0100, Jim Fehlig wrote: David Scott wrote: Hi, [added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.] On 30/04/13 16:10, Jim Fehlig wrote: David Scott wrote: The emulator path supplied can be any valid path on the system. Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either ...QEMU_XEN: meaning upstream qemu, the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork We detect the device_model_version by examining the qemu filename: if it is qemu-dm then it's the old xen-specific fork. If anything else then we assume upstream qemu (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name qemu-dm, by placing it in a separate directory. That is unfortunate. Users could have existing config with emulator/usr/bin/my-qemu-dm/emulator which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm? From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think? I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it. The intended usage model is the other way around, we expect normal users to just ask for a specific device model version and for them not to care about the specific path to the device model binary. That doesn't seem right to me. In a few years time who will care about qemu-traditional at all? People who installed Windows with it and are therefore stuck with it for the lifetime of the VM. We expect it to eventually go away but not as quickly as you might expect, for this reason. These people should use emulator/usr/lib/xen/bin/qemu-dm/emulator IMO. Seems more useful for me to be able to say use '/usr/bin/qemu-system-i386', '/usr/bin/qemu-system-arm', '/usr/lib/xen/bin/qemu-dm', '/usr/local/bin/qemu-add-my-args', etc. And if I don't specify an emulator, then pick a sane default. This is still all advanced usage, the majority of users should specify neither the version nor the path, libxl will just do the right thing for them. If the libvirt bindings is trying to insert a path in the pick a sane default case then it should stop trying to do this and just let libxl DTRT. Yes, I agree. But if a user specifies emulator/bla/bla/emulator, how should libvirt populate device_model_version? Is it expected that qemu-xen-traditional will always be the 0.10.2 version? If so, libxl could DTRT by setting device_model_version to qemu-xen-traditional when detecting the the requested emulator version in 0.10.2. BTW qemu-add-my-args can be achieved via the libxl API which has a field for extra arguments to pass the device model. Good to know, thanks. Regards, Jim [...] I would suggest that libvirt+libxl expose the version as the emulator option and not the path. That doesn't fit well with the emulator documentation That's a shame. |emulator| || ||The contents of the |emulator| element specify the fully qualified path to the device model emulator binary. The capabilities XML specifies the recommended default emulator to use for each particular domain type / architecture combination. Ian ___ Xen-devel mailing list xen-de...@lists.xen.org http://lists.xen.org/xen-devel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: Fix build when WITH_HAL is defined
Commit 7c9a2d88 missed inclusion of virstring.h in a few places when WITH_HAL is defined, causing build failures. --- Pushing under build-breaker rule. src/node_device/node_device_driver.c | 1 + src/node_device/node_device_hal.c| 1 + 2 files changed, 2 insertions(+) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 95df2e5..05dc49c 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -33,6 +33,7 @@ #include datatypes.h #include viralloc.h #include virlog.h +#include virstring.h #include node_device_conf.h #include node_device_hal.h #include node_device_driver.h diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 63245a9..8c77cc3 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -39,6 +39,7 @@ #include virlog.h #include node_device_driver.h #include virdbus.h +#include virstring.h #define VIR_FROM_THIS VIR_FROM_NODEDEV -- 1.8.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/40] Remove xen driver checks for priv-handle 0
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The Xen hypervisor driver checks for 'priv-handle 0' and
returns -1, but without raising any error. Fortunately this
code will never be executed, since the main Xen driver always
checks 'priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]' prior
to invoking any hypervisor API. Just the redundant checks
s/Just the/Just remove the/
for priv-handle
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_hypervisor.c | 98
1 file changed, 16 insertions(+), 82 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 9dbbe07..d9941ec 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1165,11 +1165,6 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int
*nparams)
char *schedulertype = NULL;
xenUnifiedPrivatePtr priv = domain-conn-privateData;
-if (priv-handle 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(priv-handle invalid));
-return NULL;
-}
if (domain-id 0) {
virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(domain is not running));
@@ -1240,11 +1235,7 @@ xenHypervisorGetSchedulerParameters(virDomainPtr
domain,
{
xenUnifiedPrivatePtr priv = domain-conn-privateData;
-if (priv-handle 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(priv-handle invalid));
-return -1;
-}
+
if (domain-id 0) {
virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(domain is not running));
@@ -1353,11 +1344,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr
domain,
NULL) 0)
return -1;
-if (priv-handle 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(priv-handle invalid));
-return -1;
-}
if (domain-id 0) {
virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(domain is not running));
@@ -2209,7 +2195,7 @@ xenHypervisorOpen(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
if (xenHypervisorInitialize() 0)
-return -1;
+return VIR_DRV_OPEN_ERROR;
priv-handle = -1;
@@ -2221,7 +2207,7 @@ xenHypervisorOpen(virConnectPtr conn,
priv-handle = ret;
-return 0;
+return VIR_DRV_OPEN_SUCCESS;
}
/**
@@ -2238,9 +2224,6 @@ xenHypervisorClose(virConnectPtr conn)
int ret;
xenUnifiedPrivatePtr priv = conn-privateData;
-if (priv-handle 0)
-return -1;
-
ret = VIR_CLOSE(priv-handle);
if (ret 0)
return -1;
@@ -2259,12 +2242,8 @@ xenHypervisorClose(virConnectPtr conn)
* Returns 0 in case of success, -1 in case of error
*/
int
-xenHypervisorGetVersion(virConnectPtr conn, unsigned long *hvVer)
+xenHypervisorGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long
*hvVer)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-
-if (priv-handle 0)
-return -1;
*hvVer = (hv_versions.hv 16) * 100 + (hv_versions.hv 0x) *
1000;
return 0;
}
@@ -2769,9 +2748,6 @@ xenHypervisorNumOfDomains(virConnectPtr conn)
int maxids = last_maxids;
xenUnifiedPrivatePtr priv = conn-privateData;
-if (priv-handle 0)
-return -1;
-
retry:
if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
virReportOOMError();
@@ -2823,9 +2799,6 @@ xenHypervisorListDomains(virConnectPtr conn, int *ids,
int maxids)
int ret, nbids, i;
xenUnifiedPrivatePtr priv = conn-privateData;
-if (priv-handle 0)
-return -1;
-
if (maxids == 0)
return 0;
@@ -2866,12 +2839,6 @@ xenHypervisorDomainGetOSType(virDomainPtr dom)
xen_getdomaininfo dominfo;
char *ostype = NULL;
-if (priv-handle 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(domain shut off or invalid));
-return NULL;
-}
-
/* HV's earlier than 3.1.0 don't include the HVM flags in guests status*/
if (hv_versions.hypervisor 2 ||
hv_versions.dom_interface 4) {
@@ -2911,9 +2878,6 @@ xenHypervisorHasDomain(virConnectPtr conn, int id)
xenUnifiedPrivatePtr priv = conn-privateData;
xen_getdomaininfo dominfo;
-if (priv-handle 0)
-return 0;
-
XEN_GETDOMAININFO_CLEAR(dominfo);
if (virXen_getdomaininfo(priv-handle, id, dominfo) 0)
@@ -2933,9 +2897,6 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int
id)
virDomainPtr ret;
char *name;
-if (priv-handle 0)
-return NULL;
-
XEN_GETDOMAININFO_CLEAR(dominfo);
if (virXen_getdomaininfo(priv-handle, id, dominfo) 0)
@@ -2967,9
Re: [libvirt] [PATCH 02/40] Remove VIR_CONNECT_RO checks from xen drivers
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Some of the Xen sub-drivers have checks against the
VIR_CONNECT_RO flag. This is not required, since such
checks are done at the top level before the driver
methods are invoked
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xm_internal.c | 24 ++--
src/xen/xs_internal.c | 2 +-
2 files changed, 3 insertions(+), 23 deletions(-)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 8ac7cb0..8580793 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -565,7 +565,7 @@ xenXMDomainSetMemory(virDomainPtr domain, unsigned long
memory)
xenXMConfCachePtr entry;
int ret = -1;
-if (domain-conn-flags VIR_CONNECT_RO || domain-id != -1 ||
+if (domain-id != -1 ||
memory 1024 * MIN_XEN_GUEST_SIZE)
The if statement can be condensed to one line now.
Otherwise, ACK.
Regards,
Jim
return -1;
@@ -604,7 +604,7 @@ xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned
long memory)
xenXMConfCachePtr entry;
int ret = -1;
-if (domain-conn-flags VIR_CONNECT_RO || domain-id != -1)
+if (domain-id != -1)
return -1;
xenUnifiedLock(priv);
@@ -686,10 +686,6 @@ xenXMDomainSetVcpusFlags(virDomainPtr domain,
VIR_DOMAIN_VCPU_CONFIG |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
-if (domain-conn-flags VIR_CONNECT_RO) {
-virReportError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
-return -1;
-}
if (domain-id != -1)
return -2;
if (flags VIR_DOMAIN_VCPU_LIVE) {
@@ -814,11 +810,6 @@ xenXMDomainPinVcpu(virDomainPtr domain,
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
return -1;
}
-if (domain-conn-flags VIR_CONNECT_RO) {
-virReportError(VIR_ERR_INVALID_ARG,
- %s, _(read only connection));
-return -1;
-}
if (domain-id != -1) {
virReportError(VIR_ERR_INVALID_ARG,
%s, _(not inactive domain));
@@ -1003,9 +994,6 @@ xenXMDomainDefineXML(virConnectPtr conn, const char *xml)
xenXMConfCachePtr entry = NULL;
xenUnifiedPrivatePtr priv = conn-privateData;
-if (conn-flags VIR_CONNECT_RO)
-return NULL;
-
xenUnifiedLock(priv);
if (!xenInotifyActive(conn) xenXMConfigCacheRefresh(conn) 0) {
@@ -1140,8 +1128,6 @@ xenXMDomainUndefine(virDomainPtr domain)
if (domain-id != -1)
return -1;
-if (domain-conn-flags VIR_CONNECT_RO)
-return -1;
xenUnifiedLock(priv);
@@ -1292,9 +1278,6 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1);
-if (domain-conn-flags VIR_CONNECT_RO)
-return -1;
-
if ((flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) ||
(domain-id != -1 flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -1386,9 +1369,6 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1);
-if (domain-conn-flags VIR_CONNECT_RO)
-return -1;
-
if ((flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) ||
(domain-id != -1 flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
index 5f0df63..393e5f9 100644
--- a/src/xen/xs_internal.c
+++ b/src/xen/xs_internal.c
@@ -133,7 +133,7 @@ virDomainDoStoreWrite(virDomainPtr domain, const char
*path, const char *value)
xenUnifiedPrivatePtr priv = domain-conn-privateData;
int ret = -1;
-if (priv-xshandle == NULL || domain-conn-flags VIR_CONNECT_RO)
+if (priv-xshandle == NULL)
return -1;
snprintf(s, 255, /local/domain/%d/%s, domain-id, path);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/40] Remove pointless GET_PRIVATE macro from Xen driver
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The Xen driver uses a macro GET_PRIVATE as a supposed shorthand
for 'xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) (conn)-privateData'.
It does not in fact save any lines of code, and obscures what is
happening. Remove it, since it adds no value.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 131
+--
1 file changed, 64 insertions(+), 67 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 0342523..2ecb86f 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -470,13 +470,10 @@ clean:
return ret;
}
-#define GET_PRIVATE(conn) \
-xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) (conn)-privateData
-
static int
xenUnifiedConnectClose(virConnectPtr conn)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
int i;
virObjectUnref(priv-caps);
@@ -508,7 +505,7 @@ unsigned long xenUnifiedVersion(void)
static const char *
xenUnifiedConnectGetType(virConnectPtr conn)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
int i;
for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
@@ -534,7 +531,7 @@ xenUnifiedConnectSupportsFeature(virConnectPtr conn
ATTRIBUTE_UNUSED, int featur
static int
xenUnifiedConnectGetVersion(virConnectPtr conn, unsigned long *hvVer)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
int i;
for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
@@ -562,7 +559,7 @@ xenUnifiedConnectIsEncrypted(virConnectPtr conn
ATTRIBUTE_UNUSED)
static int
xenUnifiedConnectIsSecure(virConnectPtr conn)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
int ret = 1;
/* All drivers are secure, except for XenD over TCP */
@@ -583,7 +580,7 @@ xenUnifiedConnectIsAlive(virConnectPtr conn
ATTRIBUTE_UNUSED)
int
xenUnifiedConnectGetMaxVcpus(virConnectPtr conn, const char *type)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
if (type STRCASENEQ(type, Xen)) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
@@ -601,7 +598,7 @@ xenUnifiedConnectGetMaxVcpus(virConnectPtr conn, const
char *type)
static int
xenUnifiedNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
return xenDaemonNodeGetInfo(conn, info);
@@ -625,7 +622,7 @@ xenUnifiedConnectGetCapabilities(virConnectPtr conn)
static int
xenUnifiedConnectListDomains(virConnectPtr conn, int *ids, int maxids)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
int ret;
/* Try xenstore. */
@@ -652,7 +649,7 @@ xenUnifiedConnectListDomains(virConnectPtr conn, int
*ids, int maxids)
static int
xenUnifiedConnectNumOfDomains(virConnectPtr conn)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
int ret;
/* Try xenstore. */
@@ -680,7 +677,7 @@ static virDomainPtr
xenUnifiedDomainCreateXML(virConnectPtr conn,
const char *xmlDesc, unsigned int flags)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
return xenDaemonCreateXML(conn, xmlDesc, flags);
@@ -694,7 +691,7 @@ xenUnifiedDomainCreateXML(virConnectPtr conn,
static virDomainPtr
xenUnifiedDomainLookupByID(virConnectPtr conn, int id)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
virDomainPtr ret;
/* Reset any connection-level errors in virterror first, in case
@@ -725,7 +722,7 @@ static virDomainPtr
xenUnifiedDomainLookupByUUID(virConnectPtr conn,
const unsigned char *uuid)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
virDomainPtr ret;
/* Reset any connection-level errors in virterror first, in case
@@ -763,7 +760,7 @@ static virDomainPtr
xenUnifiedDomainLookupByName(virConnectPtr conn,
const char *name)
{
-GET_PRIVATE(conn);
+xenUnifiedPrivatePtr priv = conn-privateData;
virDomainPtr ret;
/* Reset any connection-level errors in virterror first, in case
@@ -818,7 +815,7 @@ xenUnifiedDomainIsActive(virDomainPtr dom)
static int
xenUnifiedDomainIsPersistent(virDomainPtr dom)
{
-GET_PRIVATE(dom-conn);
+xenUnifiedPrivatePtr priv = dom-conn-privateData;
virDomainPtr currdom = NULL;
int ret = -1;
@@ -875,7 +872,7 @@ xenUnifiedDomainIsUpdated(virDomainPtr dom
ATTRIBUTE_UNUSED)
static int
xenUnifiedDomainSuspend(virDomainPtr dom)
{
-
Re: [libvirt] [PATCH 04/40] Simplify opening of Xen drivers
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Since the Xen driver was changed to only execute inside libvirtd,
there is no scenario in which it will be opened from a non-privileged
context. This all the code dealing with opening the sub-drivers can
s/This/Thus/ ?
be simplified to assume that they are always privileged.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 148
+--
src/xen/xen_driver.h | 1 -
src/xen/xen_hypervisor.c | 9 ++-
src/xen/xen_hypervisor.h | 2 +-
src/xen/xen_inotify.c| 8 +--
src/xen/xen_inotify.h| 11 ++--
src/xen/xend_internal.c | 9 ++-
src/xen/xend_internal.h | 4 +-
src/xen/xm_internal.c| 5 +-
src/xen/xm_internal.h| 4 +-
src/xen/xs_internal.c| 5 +-
src/xen/xs_internal.h| 2 +-
12 files changed, 91 insertions(+), 117 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 2ecb86f..b7f1ad4 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -86,14 +86,9 @@ static struct xenUnifiedDriver const * const
drivers[XEN_UNIFIED_NR_DRIVERS] = {
[XEN_UNIFIED_XEND_OFFSET] = xenDaemonDriver,
[XEN_UNIFIED_XS_OFFSET] = xenStoreDriver,
[XEN_UNIFIED_XM_OFFSET] = xenXMDriver,
-#if WITH_XEN_INOTIFY
-[XEN_UNIFIED_INOTIFY_OFFSET] = xenInotifyDriver,
-#endif
Looks like this was never used, so just removing it right? But there are
a lot of loops in this file with 'drivers[i]-', where it might be
possible that i == XEN_UNIFIED_INOTIFY_OFFSET?
};
-#if defined WITH_LIBVIRTD || defined __sun
static bool inside_daemon = false;
-#endif
/**
* xenNumaInit:
@@ -200,14 +195,14 @@ done:
return res;
}
-#ifdef WITH_LIBVIRTD
-
static int
-xenUnifiedStateInitialize(bool privileged ATTRIBUTE_UNUSED,
+xenUnifiedStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
{
-inside_daemon = true;
+/* Don't allow driver to work in non-root libvirtd */
+if (privileged)
+inside_daemon = true;
Seems the name 'inside_daemon' is no longer appropriate. Should it be
something like 'is_privileged'?
return 0;
}
@@ -216,8 +211,6 @@ static virStateDriver state_driver = {
.stateInitialize = xenUnifiedStateInitialize,
};
-#endif
-
/*- Dispatch functions. -*/
/* These dispatch functions follow the model used historically
@@ -298,18 +291,15 @@ xenDomainXMLConfInit(void)
static virDrvOpenStatus
xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned
int flags)
{
-int i, ret = VIR_DRV_OPEN_DECLINED;
xenUnifiedPrivatePtr priv;
char ebuf[1024];
-#ifdef __sun
/*
* Only the libvirtd instance can open this driver.
* Everything else falls back to the remote driver.
*/
if (!inside_daemon)
return VIR_DRV_OPEN_DECLINED;
-#endif
if (conn-uri == NULL) {
if (!xenUnifiedProbe())
@@ -379,110 +369,108 @@ xenUnifiedConnectOpen(virConnectPtr conn,
virConnectAuthPtr auth, unsigned int f
priv-xshandle = NULL;
-/* Hypervisor is only run with privilege required to succeed */
-if (xenHavePrivilege()) {
-VIR_DEBUG(Trying hypervisor sub-driver);
-if (xenHypervisorOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
-VIR_DEBUG(Activated hypervisor sub-driver);
-priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1;
-} else {
-goto fail;
-}
-}
+/* Hypervisor required to succeed */
+VIR_DEBUG(Trying hypervisor sub-driver);
+if (xenHypervisorOpen(conn, auth, flags) 0)
+goto error;
+VIR_DEBUG(Activated hypervisor sub-driver);
+priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1;
-/* XenD is required to succeed if privileged */
+/* XenD is required to succeed */
VIR_DEBUG(Trying XenD sub-driver);
-if (xenDaemonOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
-VIR_DEBUG(Activated XenD sub-driver);
-priv-opened[XEN_UNIFIED_XEND_OFFSET] = 1;
-
-/* XenD is active, so try the xm xs drivers too, both requird to
- * succeed if root, optional otherwise */
-if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3) {
-VIR_DEBUG(Trying XM sub-driver);
-if (xenXMOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
-VIR_DEBUG(Activated XM sub-driver);
-priv-opened[XEN_UNIFIED_XM_OFFSET] = 1;
-}
-}
-VIR_DEBUG(Trying XS sub-driver);
-if (xenStoreOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
-VIR_DEBUG(Activated XS sub-driver);
-priv-opened[XEN_UNIFIED_XS_OFFSET] = 1;
-}
Re: [libvirt] [PATCH 05/40] Simplify the Xen get type driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
There is no point iterating over sub-drivers since the user
would not have a virConnectPtr instance at all if opening
the drivers failed. Just return 'Xen' immediately.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index b7f1ad4..109a074 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -491,16 +491,9 @@ unsigned long xenUnifiedVersion(void)
static const char *
-xenUnifiedConnectGetType(virConnectPtr conn)
+xenUnifiedConnectGetType(virConnectPtr conn ATTRIBUTE_UNUSED)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i])
-return Xen;
-
-return NULL;
+return Xen;
}
/* Which features are supported by this driver? */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/40] Simplify the Xen get version driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The hypervisor driver is mandatory, so the the call to
xenHypervisorGetVersion must always succeed. Thus there
is no need to ever run xenDaemonGetVersion
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 11 +--
src/xen/xen_driver.h | 1 -
src/xen/xen_hypervisor.c | 1 -
src/xen/xend_internal.c | 31 ---
src/xen/xend_internal.h | 1 -
5 files changed, 1 insertion(+), 44 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 109a074..951f6e7 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -512,16 +512,7 @@ xenUnifiedConnectSupportsFeature(virConnectPtr conn
ATTRIBUTE_UNUSED, int featur
static int
xenUnifiedConnectGetVersion(virConnectPtr conn, unsigned long *hvVer)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenVersion
-drivers[i]-xenVersion(conn, hvVer) == 0)
-return 0;
-
-return -1;
+return xenHypervisorGetVersion(conn, hvVer);
}
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 70c1226..803fee7 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -93,7 +93,6 @@ extern int xenRegister (void);
* structure with direct calls in xen_unified.c.
*/
struct xenUnifiedDriver {
-virDrvConnectGetVersion xenVersion;
virDrvConnectGetHostname xenGetHostname;
virDrvDomainSuspend xenDomainSuspend;
virDrvDomainResume xenDomainResume;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 6b41898..012cb0e 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -880,7 +880,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
struct xenUnifiedDriver xenHypervisorDriver = {
-.xenVersion = xenHypervisorGetVersion,
.xenDomainSuspend = xenHypervisorPauseDomain,
.xenDomainResume = xenHypervisorResumeDomain,
.xenDomainDestroyFlags = xenHypervisorDestroyDomainFlags,
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index eb3e63e..eb11408 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1861,36 +1861,6 @@ xenDaemonNodeGetTopology(virConnectPtr conn,
virCapsPtr caps)
return ret;
}
-/**
- * xenDaemonGetVersion:
- * @conn: pointer to the Xen Daemon block
- * @hvVer: return value for the version of the running hypervisor (OUT)
- *
- * Get the version level of the Hypervisor running.
- *
- * Returns -1 in case of error, 0 otherwise. if the version can't be
- *extracted by lack of capacities returns 0 and @hvVer is 0, otherwise
- *@hvVer value is major * 1,000,000 + minor * 1,000 + release
- */
-int
-xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer)
-{
-struct sexpr *root;
-int major, minor;
-unsigned long version;
-
-root = sexpr_get(conn, /xend/node/);
-if (root == NULL)
-return -1;
-
-major = sexpr_int(root, node/xen_major);
-minor = sexpr_int(root, node/xen_minor);
-sexpr_free(root);
-version = major * 100 + minor * 1000;
-*hvVer = version;
-return 0;
-}
-
/**
* xenDaemonListDomains:
@@ -3652,7 +3622,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
}
struct xenUnifiedDriver xenDaemonDriver = {
-.xenVersion = xenDaemonGetVersion,
.xenDomainSuspend = xenDaemonDomainSuspend,
.xenDomainResume = xenDaemonDomainResume,
.xenDomainShutdown = xenDaemonDomainShutdown,
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index e5c0896..41d8341 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -98,7 +98,6 @@ xenDaemonDomainFetch(virConnectPtr xend,
int xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth,
unsigned int flags);
int xenDaemonClose(virConnectPtr conn);
-int xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer);
int xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);
int xenDaemonNodeGetTopology(virConnectPtr conn, virCapsPtr caps);
int xenDaemonDomainSuspend(virDomainPtr domain);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/40] Simplify the Xen get max vcpus / node get info driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally call into xenHypervisorGetMaxVcpus and
xenDaemonNodeGetInfo respectively, since those drivers
are both mandatory
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 951f6e7..b6cf6ca 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -552,29 +552,18 @@ xenUnifiedConnectIsAlive(virConnectPtr conn
ATTRIBUTE_UNUSED)
int
xenUnifiedConnectGetMaxVcpus(virConnectPtr conn, const char *type)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-
if (type STRCASENEQ(type, Xen)) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
return -1;
}
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET])
-return xenHypervisorGetMaxVcpus(conn, type);
-else {
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
-}
+return xenHypervisorGetMaxVcpus(conn, type);
}
static int
xenUnifiedNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonNodeGetInfo(conn, info);
-return -1;
+return xenDaemonNodeGetInfo(conn, info);
}
static char *
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/40] Simplify the Xen count/list domains driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The XenStore driver is mandatory, so it can be used unconditonally
for the xenUnifiedConnectListDomains xenUnifiedConnectNumOfDomains
drivers. Delete the unused XenD and Hypervisor driver code for
listing / counting domains
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 46 +
src/xen/xen_hypervisor.c | 101
---
src/xen/xen_hypervisor.h | 4 --
src/xen/xend_internal.c | 81 -
src/xen/xend_internal.h | 2 -
src/xen/xs_internal.c| 8
6 files changed, 2 insertions(+), 240 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index b6cf6ca..25fb7bb 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -583,55 +583,13 @@ xenUnifiedConnectGetCapabilities(virConnectPtr conn)
static int
xenUnifiedConnectListDomains(virConnectPtr conn, int *ids, int maxids)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-int ret;
-
-/* Try xenstore. */
-if (priv-opened[XEN_UNIFIED_XS_OFFSET]) {
-ret = xenStoreListDomains(conn, ids, maxids);
-if (ret = 0) return ret;
-}
-
-/* Try HV. */
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorListDomains(conn, ids, maxids);
-if (ret = 0) return ret;
-}
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonListDomains(conn, ids, maxids);
-if (ret = 0) return ret;
-}
-
-return -1;
+return xenStoreListDomains(conn, ids, maxids);
Probably not likely, but what if xenStoreListDomains() failed, e.g.
xshandle somehow became stale? The existing code would try the other
drivers if xenstore one failed.
Regards,
Jim
}
static int
xenUnifiedConnectNumOfDomains(virConnectPtr conn)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-int ret;
-
-/* Try xenstore. */
-if (priv-opened[XEN_UNIFIED_XS_OFFSET]) {
-ret = xenStoreNumOfDomains(conn);
-if (ret = 0) return ret;
-}
-
-/* Try HV. */
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorNumOfDomains(conn);
-if (ret = 0) return ret;
-}
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonNumOfDomains(conn);
-if (ret = 0) return ret;
-}
-
-return -1;
+return xenStoreNumOfDomains(conn);
}
static virDomainPtr
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 012cb0e..2068a8a 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -2729,107 +2729,6 @@ xenHypervisorGetCapabilities(virConnectPtr conn)
}
-/**
- * xenHypervisorNumOfDomains:
- * @conn: pointer to the connection block
- *
- * Provides the number of active domains.
- *
- * Returns the number of domain found or -1 in case of error
- */
-int
-xenHypervisorNumOfDomains(virConnectPtr conn)
-{
-xen_getdomaininfolist dominfos;
-int ret, nbids;
-static int last_maxids = 2;
-int maxids = last_maxids;
-xenUnifiedPrivatePtr priv = conn-privateData;
-
- retry:
-if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
-virReportOOMError();
-return -1;
-}
-
-XEN_GETDOMAININFOLIST_CLEAR(dominfos, maxids);
-
-ret = virXen_getdomaininfolist(priv-handle, 0, maxids, dominfos);
-
-XEN_GETDOMAININFOLIST_FREE(dominfos);
-
-if (ret 0)
-return -1;
-
-nbids = ret;
-/* Can't possibly have more than 65,000 concurrent guests
- * so limit how many times we try, to avoid increasing
- * without bound thus allocating all of system memory !
- * XXX I'll regret this comment in a few years time ;-)
- */
-if (nbids == maxids) {
-if (maxids 65000) {
-last_maxids *= 2;
-maxids *= 2;
-goto retry;
-}
-nbids = -1;
-}
-if ((nbids 0) || (nbids maxids))
-return -1;
-return nbids;
-}
-
-/**
- * xenHypervisorListDomains:
- * @conn: pointer to the connection block
- * @ids: array to collect the list of IDs of active domains
- * @maxids: size of @ids
- *
- * Collect the list of active domains, and store their ID in @maxids
- *
- * Returns the number of domain found or -1 in case of error
- */
-int
-xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids)
-{
-xen_getdomaininfolist dominfos;
-int ret, nbids, i;
-xenUnifiedPrivatePtr priv = conn-privateData;
-
-if (maxids == 0)
-return 0;
-
-if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
-virReportOOMError();
-return -1;
-}
-
-XEN_GETDOMAININFOLIST_CLEAR(dominfos, maxids);
-memset(ids,
Re: [libvirt] [PATCH 09/40] Simplify the Xen domain create driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally call xenDaemonCreateXML in the
xenUnifiedDomainCreateXML driver, since the XenD
driver is always present.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 6 ++
src/xen/xend_internal.c | 4 +---
src/xen/xend_internal.h | 3 +--
3 files changed, 4 insertions(+), 9 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 25fb7bb..82058b7 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -596,11 +596,9 @@ static virDomainPtr
xenUnifiedDomainCreateXML(virConnectPtr conn,
const char *xmlDesc, unsigned int flags)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
+virCheckFlags(0, NULL);
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonCreateXML(conn, xmlDesc, flags);
-return NULL;
+return xenDaemonCreateXML(conn, xmlDesc);
}
/* Assumption made in underlying drivers:
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 952eb3f..2e6a47e 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2288,7 +2288,7 @@ xenDaemonLookupByUUID(virConnectPtr conn, const
unsigned char *uuid)
* Returns a new domain object or NULL in case of failure
*/
virDomainPtr
-xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int
flags)
+xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc)
{
int ret;
char *sexpr;
@@ -2296,8 +2296,6 @@ xenDaemonCreateXML(virConnectPtr conn, const char
*xmlDesc, unsigned int flags)
xenUnifiedPrivatePtr priv = conn-privateData;
virDomainDefPtr def;
-virCheckFlags(0, NULL);
-
if (!(def = virDomainDefParseString(xmlDesc, priv-caps, priv-xmlopt,
1 VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index f6760a2..5f82f04 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -152,8 +152,7 @@ int xenDaemonDomainSetAutostart (virDomainPtr
domain,
extern struct xenUnifiedDriver xenDaemonDriver;
int xenDaemonInit (void);
-virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc,
-unsigned int flags);
+virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc);
virDomainPtr xenDaemonLookupByID(virConnectPtr conn, int id);
virDomainPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char
*uuid);
virDomainPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/40] Simplify the Xen count/list domains driver methods
Jim Fehlig wrote:
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The XenStore driver is mandatory, so it can be used unconditonally
for the xenUnifiedConnectListDomains xenUnifiedConnectNumOfDomains
drivers. Delete the unused XenD and Hypervisor driver code for
listing / counting domains
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 46 +
src/xen/xen_hypervisor.c | 101
---
src/xen/xen_hypervisor.h | 4 --
src/xen/xend_internal.c | 81 -
src/xen/xend_internal.h | 2 -
src/xen/xs_internal.c| 8
6 files changed, 2 insertions(+), 240 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index b6cf6ca..25fb7bb 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -583,55 +583,13 @@ xenUnifiedConnectGetCapabilities(virConnectPtr conn)
static int
xenUnifiedConnectListDomains(virConnectPtr conn, int *ids, int maxids)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-int ret;
-
-/* Try xenstore. */
-if (priv-opened[XEN_UNIFIED_XS_OFFSET]) {
-ret = xenStoreListDomains(conn, ids, maxids);
-if (ret = 0) return ret;
-}
-
-/* Try HV. */
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorListDomains(conn, ids, maxids);
-if (ret = 0) return ret;
-}
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonListDomains(conn, ids, maxids);
-if (ret = 0) return ret;
-}
-
-return -1;
+return xenStoreListDomains(conn, ids, maxids);
Probably not likely, but what if xenStoreListDomains() failed, e.g.
xshandle somehow became stale? The existing code would try the other
drivers if xenstore one failed.
I started to make a similar comment for patch 10, but then re-read your
cover letter and realize this is intentional. While I agree with the
direct invocation in 10, and probably others I've yet to review, this
may be a case where we actually want to try something other than
xenstore. I seem to recall an issue long ago where trying multiple
drivers helped when there were state inconsistencies in the xen tools.
But then again, maybe it is best to simply fail instead of propagating
those inconsistencies?
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally invoke the xenHypervisorLookupDomainByID,
xenHypervisorLookupDomainByUUID or xenDaemonLookupByName
for looking up domains. Fallback to xenXMDomainLookupByUUID
and xenXMDomainLookupByName for legacy XenD without inactive
domain support
Do you think there are any Xen installations running such an old xend
toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the
XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 99
+++--
src/xen/xend_internal.c | 89
src/xen/xend_internal.h | 14 ---
src/xen/xs_internal.c | 62 ---
src/xen/xs_internal.h | 2 -
5 files changed, 22 insertions(+), 244 deletions(-)
I spent some time testing this one and didn't notice any problems.
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 82058b7..080045c 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -601,38 +601,17 @@ xenUnifiedDomainCreateXML(virConnectPtr conn,
return xenDaemonCreateXML(conn, xmlDesc);
}
-/* Assumption made in underlying drivers:
- * If the domain is not found and there is no other error, then
- * the Lookup* functions return a NULL but do not set virterror.
- */
static virDomainPtr
xenUnifiedDomainLookupByID(virConnectPtr conn, int id)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-virDomainPtr ret;
+virDomainPtr ret = NULL;
-/* Reset any connection-level errors in virterror first, in case
- * there is one hanging around from a previous call.
- */
-virConnResetLastError(conn);
+ret = xenHypervisorLookupDomainByID(conn, id);
-/* Try hypervisor/xenstore combo. */
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorLookupDomainByID(conn, id);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonLookupByID(conn, id);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
-/* Not found. */
-virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
-return NULL;
+return ret;
}
static virDomainPtr
@@ -642,35 +621,20 @@ xenUnifiedDomainLookupByUUID(virConnectPtr conn,
xenUnifiedPrivatePtr priv = conn-privateData;
virDomainPtr ret;
-/* Reset any connection-level errors in virterror first, in case
- * there is one hanging around from a previous call.
- */
-virConnResetLastError(conn);
-
-/* Try hypervisor/xenstore combo. */
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorLookupDomainByUUID(conn, uuid);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonLookupByUUID(conn, uuid);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
+ret = xenHypervisorLookupDomainByUUID(conn, uuid);
/* Try XM for inactive domains. */
-if (priv-opened[XEN_UNIFIED_XM_OFFSET]) {
-ret = xenXMDomainLookupByUUID(conn, uuid);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
+if (!ret) {
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3)
+ret = xenXMDomainLookupByUUID(conn, uuid);
+else
+return xenDaemonLookupByUUID(conn, uuid);
}
-/* Not found. */
-virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
-return NULL;
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
}
static virDomainPtr
@@ -680,35 +644,16 @@ xenUnifiedDomainLookupByName(virConnectPtr conn,
xenUnifiedPrivatePtr priv = conn-privateData;
virDomainPtr ret;
-/* Reset any connection-level errors in virterror first, in case
- * there is one hanging around from a previous call.
- */
-virConnResetLastError(conn);
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonLookupByName(conn, name);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
-
-/* Try xenstore for inactive domains. */
-if (priv-opened[XEN_UNIFIED_XS_OFFSET]) {
-ret = xenStoreLookupByName(conn, name);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
+ret = xenDaemonLookupByName(conn, name);
/* Try XM for inactive
Re: [libvirt] [PATCH 11/40] Simplify the Xen domain is persistent driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally call xenDaemonLookupByUUID, since the XenD
driver must always be present.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 41 +++--
1 file changed, 19 insertions(+), 22 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 080045c..5f296ad 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -691,29 +691,26 @@ xenUnifiedDomainIsPersistent(virDomainPtr dom)
ret = 0;
} else {
/* New Xen with inactive domain management */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-currdom = xenDaemonLookupByUUID(dom-conn, dom-uuid);
-if (currdom) {
-if (currdom-id == -1) {
-/* If its inactive, then trivially, it must be
persistent */
-ret = 1;
-} else {
-char *path;
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-/* If its running there's no official way to tell, so we
- * go behind xend's back look at the config dir */
-
-virUUIDFormat(dom-uuid, uuidstr);
-if (virAsprintf(path, %s/%s, XEND_DOMAINS_DIR,
uuidstr) 0) {
-virReportOOMError();
-goto done;
-}
-if (access(path, R_OK) == 0)
-ret = 1;
-else if (errno == ENOENT)
-ret = 0;
+currdom = xenDaemonLookupByUUID(dom-conn, dom-uuid);
+if (currdom) {
+if (currdom-id == -1) {
+/* If its inactive, then trivially, it must be persistent */
+ret = 1;
+} else {
+char *path;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+/* If its running there's no official way to tell, so we
+ * go behind xend's back look at the config dir */
+virUUIDFormat(dom-uuid, uuidstr);
+if (virAsprintf(path, %s/%s, XEND_DOMAINS_DIR, uuidstr)
0) {
+virReportOOMError();
+goto done;
}
+if (access(path, R_OK) == 0)
+ret = 1;
+else if (errno == ENOENT)
+ret = 0;
}
}
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/40] Simplify the Xen domain suspend/resume driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Update xenUnifiedDomainSuspend and xenUnifiedDomainResume to
unconditionally invoke the XenD APIs for suspend/resume. Delete
the impls in the hypervisor driver which was unreachable.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 38 +-
src/xen/xen_driver.h | 2 -
src/xen/xen_hypervisor.c | 125
---
src/xen/xen_hypervisor.h | 4 --
src/xen/xend_internal.c | 2 -
5 files changed, 2 insertions(+), 169 deletions(-)
I wonder if the hypervisor driver code was ever used...
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 5f296ad..b6d5124 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -731,47 +731,13 @@ xenUnifiedDomainIsUpdated(virDomainPtr dom
ATTRIBUTE_UNUSED)
static int
xenUnifiedDomainSuspend(virDomainPtr dom)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-
-/* Try non-hypervisor methods first, then hypervisor direct method
- * as a last resort.
- */
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (i != XEN_UNIFIED_HYPERVISOR_OFFSET
-priv-opened[i]
-drivers[i]-xenDomainSuspend
-drivers[i]-xenDomainSuspend(dom) == 0)
-return 0;
-
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]
-xenHypervisorPauseDomain(dom) == 0)
-return 0;
-
-return -1;
+return xenDaemonDomainSuspend(dom);
}
static int
xenUnifiedDomainResume(virDomainPtr dom)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-
-/* Try non-hypervisor methods first, then hypervisor direct method
- * as a last resort.
- */
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (i != XEN_UNIFIED_HYPERVISOR_OFFSET
-priv-opened[i]
-drivers[i]-xenDomainResume
-drivers[i]-xenDomainResume(dom) == 0)
-return 0;
-
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]
-xenHypervisorResumeDomain(dom) == 0)
-return 0;
-
-return -1;
+return xenDaemonDomainResume(dom);
}
static int
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 803fee7..ac38b19 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -94,8 +94,6 @@ extern int xenRegister (void);
*/
struct xenUnifiedDriver {
virDrvConnectGetHostname xenGetHostname;
-virDrvDomainSuspend xenDomainSuspend;
-virDrvDomainResume xenDomainResume;
virDrvDomainShutdown xenDomainShutdown;
virDrvDomainReboot xenDomainReboot;
virDrvDomainDestroyFlags xenDomainDestroyFlags;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 2068a8a..71212eb 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -880,8 +880,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
struct xenUnifiedDriver xenHypervisorDriver = {
-.xenDomainSuspend = xenHypervisorPauseDomain,
-.xenDomainResume = xenHypervisorResumeDomain,
.xenDomainDestroyFlags = xenHypervisorDestroyDomainFlags,
.xenDomainGetOSType = xenHypervisorDomainGetOSType,
.xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
@@ -1486,83 +1484,6 @@ xenHypervisorDomainInterfaceStats(virDomainPtr dom,
#endif
}
-/**
- * virXen_pausedomain:
- * @handle: the hypervisor handle
- * @id: the domain id
- *
- * Do a low level hypercall to pause the domain
- *
- * Returns 0 or -1 in case of failure
- */
-static int
-virXen_pausedomain(int handle, int id)
-{
-int ret = -1;
-
-if (hv_versions.hypervisor 1) {
-xen_op_v2_dom op;
-
-memset(op, 0, sizeof(op));
-op.cmd = XEN_V2_OP_PAUSEDOMAIN;
-op.domain = (domid_t) id;
-ret = xenHypervisorDoV2Dom(handle, op);
-} else if (hv_versions.hypervisor == 1) {
-xen_op_v1 op;
-
-memset(op, 0, sizeof(op));
-op.cmd = XEN_V1_OP_PAUSEDOMAIN;
-op.u.domain.domain = (domid_t) id;
-ret = xenHypervisorDoV1Op(handle, op);
-} else if (hv_versions.hypervisor == 0) {
-xen_op_v0 op;
-
-memset(op, 0, sizeof(op));
-op.cmd = XEN_V0_OP_PAUSEDOMAIN;
-op.u.domain.domain = (domid_t) id;
-ret = xenHypervisorDoV0Op(handle, op);
-}
-return ret;
-}
-
-/**
- * virXen_unpausedomain:
- * @handle: the hypervisor handle
- * @id: the domain id
- *
- * Do a low level hypercall to unpause the domain
- *
- * Returns 0 or -1 in case of failure
- */
-static int
-virXen_unpausedomain(int handle, int id)
-{
-int ret = -1;
-
-if (hv_versions.hypervisor 1) {
-xen_op_v2_dom op;
-
-memset(op, 0, sizeof(op));
-op.cmd =
Re: [libvirt] [PATCH 14/40] Simplify the Xen domain destroy driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally call the xenDaemonDomainDestroyFlags API
since the XenD driver is always available.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 19 +---
src/xen/xen_driver.h | 1 -
src/xen/xen_hypervisor.c | 76
src/xen/xen_hypervisor.h | 5
src/xen/xend_internal.c | 13 ++---
src/xen/xend_internal.h | 2 +-
src/xen/xm_internal.c| 2 +-
7 files changed, 6 insertions(+), 112 deletions(-)
Lots of nice cleanup in this series. ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index db13438..7827d70 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -767,26 +767,9 @@ static int
xenUnifiedDomainDestroyFlags(virDomainPtr dom,
unsigned int flags)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-
virCheckFlags(0, -1);
-/* Try non-hypervisor methods first, then hypervisor direct method
- * as a last resort.
- */
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (i != XEN_UNIFIED_HYPERVISOR_OFFSET
-priv-opened[i]
-drivers[i]-xenDomainDestroyFlags
-drivers[i]-xenDomainDestroyFlags(dom, flags) == 0)
-return 0;
-
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]
-xenHypervisorDestroyDomainFlags(dom, flags) == 0)
-return 0;
-
-return -1;
+return xenDaemonDomainDestroy(dom);
}
static int
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index b77067d..aff68f2 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -94,7 +94,6 @@ extern int xenRegister (void);
*/
struct xenUnifiedDriver {
virDrvConnectGetHostname xenGetHostname;
-virDrvDomainDestroyFlags xenDomainDestroyFlags;
virDrvDomainGetOSType xenDomainGetOSType;
virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 71212eb..244bdee 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -609,13 +609,6 @@ struct xen_v0_domainop {
typedef struct xen_v0_domainop xen_v0_domainop;
/*
- * The information for a destroydomain system hypercall
- */
-#define XEN_V0_OP_DESTROYDOMAIN 9
-#define XEN_V1_OP_DESTROYDOMAIN 9
-#define XEN_V2_OP_DESTROYDOMAIN 2
-
-/*
* The information for a pausedomain system hypercall
*/
#define XEN_V0_OP_PAUSEDOMAIN10
@@ -880,7 +873,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
struct xenUnifiedDriver xenHypervisorDriver = {
-.xenDomainDestroyFlags = xenHypervisorDestroyDomainFlags,
.xenDomainGetOSType = xenHypervisorDomainGetOSType,
.xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
.xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
@@ -1486,45 +1478,6 @@ xenHypervisorDomainInterfaceStats(virDomainPtr dom,
/**
- * virXen_destroydomain:
- * @handle: the hypervisor handle
- * @id: the domain id
- *
- * Do a low level hypercall to destroy the domain
- *
- * Returns 0 or -1 in case of failure
- */
-static int
-virXen_destroydomain(int handle, int id)
-{
-int ret = -1;
-
-if (hv_versions.hypervisor 1) {
-xen_op_v2_dom op;
-
-memset(op, 0, sizeof(op));
-op.cmd = XEN_V2_OP_DESTROYDOMAIN;
-op.domain = (domid_t) id;
-ret = xenHypervisorDoV2Dom(handle, op);
-} else if (hv_versions.hypervisor == 1) {
-xen_op_v1 op;
-
-memset(op, 0, sizeof(op));
-op.cmd = XEN_V1_OP_DESTROYDOMAIN;
-op.u.domain.domain = (domid_t) id;
-ret = xenHypervisorDoV1Op(handle, op);
-} else if (hv_versions.hypervisor == 0) {
-xen_op_v0 op;
-
-memset(op, 0, sizeof(op));
-op.cmd = XEN_V0_OP_DESTROYDOMAIN;
-op.u.domain.domain = (domid_t) id;
-ret = xenHypervisorDoV0Op(handle, op);
-}
-return ret;
-}
-
-/**
* virXen_setmaxmem:
* @handle: the hypervisor handle
* @id: the domain id
@@ -3064,35 +3017,6 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn,
/**
- * xenHypervisorDestroyDomainFlags:
- * @domain: pointer to the domain block
- * @flags: an OR'ed set of virDomainDestroyFlagsValues
- *
- * Do a hypervisor call to destroy the given domain
- *
- * Calling this function with no @flags set (equal to zero)
- * is equivalent to calling xenHypervisorDestroyDomain.
- *
- * Returns 0 in case of success, -1 in case of error.
- */
-int
-xenHypervisorDestroyDomainFlags(virDomainPtr domain, unsigned int flags)
-{
-int ret;
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-
-
Re: [libvirt] [PATCH 15/40] Simplify the Xen domain get OS type driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make xenUnifiedDomainGetOSType directly call either the
xenHypervisorDomainGetOSType or xenDaemonDomainGetOSType
method depending on whether the domain is active or not.
Useful to add a note about removing the unused code in the xenstore
driver, as you did in the other patches.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 19 ++-
src/xen/xen_driver.h | 1 -
src/xen/xen_hypervisor.c | 5 +--
src/xen/xend_internal.c | 7 +---
src/xen/xend_internal.h | 2 ++
src/xen/xs_internal.c| 85
6 files changed, 15 insertions(+), 104 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 7827d70..8ee3c4c 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -782,18 +782,21 @@ static char *
xenUnifiedDomainGetOSType(virDomainPtr dom)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-char *ret;
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainGetOSType) {
-ret = drivers[i]-xenDomainGetOSType(dom);
-if (ret) return ret;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to query OS type for inactive domain));
+return NULL;
+} else {
+return xenDaemonDomainGetOSType(dom);
}
-
-return NULL;
+} else {
+return xenHypervisorDomainGetOSType(dom);
+}
}
+
static unsigned long long
xenUnifiedDomainGetMaxMemory(virDomainPtr dom)
{
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index aff68f2..3ac2912 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -94,7 +94,6 @@ extern int xenRegister (void);
*/
struct xenUnifiedDriver {
virDrvConnectGetHostname xenGetHostname;
-virDrvDomainGetOSType xenDomainGetOSType;
virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
virDrvDomainSetMemory xenDomainSetMemory;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 244bdee..8636d52 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -873,7 +873,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
struct xenUnifiedDriver xenHypervisorDriver = {
-.xenDomainGetOSType = xenHypervisorDomainGetOSType,
.xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
.xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
.xenDomainGetInfo = xenHypervisorGetDomainInfo,
@@ -2613,9 +2612,7 @@ xenHypervisorDomainGetOSType(virDomainPtr dom)
/* HV's earlier than 3.1.0 don't include the HVM flags in guests status*/
if (hv_versions.hypervisor 2 ||
hv_versions.dom_interface 4) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(unsupported in dom interface 4));
-return NULL;
+return xenDaemonDomainGetOSType(dom);
}
XEN_GETDOMAININFO_CLEAR(dominfo);
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index c759636..75c1514 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1373,15 +1373,11 @@ xenDaemonDomainDestroy(virDomainPtr domain)
* Returns the new string or NULL in case of error, the string must be
* freed by the caller.
*/
-static char *
+char *
xenDaemonDomainGetOSType(virDomainPtr domain)
{
char *type;
struct sexpr *root;
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-
-if (domain-id 0 priv-xendConfigVersion
XEND_CONFIG_VERSION_3_0_4)
-return NULL;
/* can we ask for a subset ? worth it ? */
root = sexpr_get(domain-conn, /xend/domain/%s?detail=1, domain-name);
@@ -3441,7 +3437,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
}
struct xenUnifiedDriver xenDaemonDriver = {
-.xenDomainGetOSType = xenDaemonDomainGetOSType,
.xenDomainGetMaxMemory = xenDaemonDomainGetMaxMemory,
.xenDomainSetMaxMemory = xenDaemonDomainSetMaxMemory,
.xenDomainSetMemory = xenDaemonDomainSetMemory,
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index d393ec8..9681068 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -108,6 +108,8 @@ char *xenDaemonDomainGetXMLDesc(virDomainPtr domain,
unsigned int flags,
unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain);
char **xenDaemonListDomainsOld(virConnectPtr xend);
+char *xenDaemonDomainGetOSType(virDomainPtr domain);
+
virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr);
int
Re: [libvirt] [PATCH 15/40] Simplify the Xen domain get OS type driver method
Jim Fehlig wrote:
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make xenUnifiedDomainGetOSType directly call either the
xenHypervisorDomainGetOSType or xenDaemonDomainGetOSType
method depending on whether the domain is active or not.
Useful to add a note about removing the unused code in the xenstore
driver, as you did in the other patches.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 19 ++-
src/xen/xen_driver.h | 1 -
src/xen/xen_hypervisor.c | 5 +--
src/xen/xend_internal.c | 7 +---
src/xen/xend_internal.h | 2 ++
src/xen/xs_internal.c| 85
6 files changed, 15 insertions(+), 104 deletions(-)
ACK.
Forgot to mention one little nit below
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 7827d70..8ee3c4c 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -782,18 +782,21 @@ static char *
xenUnifiedDomainGetOSType(virDomainPtr dom)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-char *ret;
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainGetOSType) {
-ret = drivers[i]-xenDomainGetOSType(dom);
-if (ret) return ret;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to query OS type for inactive
domain));
+return NULL;
+} else {
+return xenDaemonDomainGetOSType(dom);
}
-
-return NULL;
+} else {
+return xenHypervisorDomainGetOSType(dom);
+}
}
+
Spurious whitespace.
Regards,
Jim
static unsigned long long
xenUnifiedDomainGetMaxMemory(virDomainPtr dom)
{
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index aff68f2..3ac2912 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -94,7 +94,6 @@ extern int xenRegister (void);
*/
struct xenUnifiedDriver {
virDrvConnectGetHostname xenGetHostname;
-virDrvDomainGetOSType xenDomainGetOSType;
virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
virDrvDomainSetMemory xenDomainSetMemory;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 244bdee..8636d52 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -873,7 +873,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
struct xenUnifiedDriver xenHypervisorDriver = {
-.xenDomainGetOSType = xenHypervisorDomainGetOSType,
.xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
.xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
.xenDomainGetInfo = xenHypervisorGetDomainInfo,
@@ -2613,9 +2612,7 @@ xenHypervisorDomainGetOSType(virDomainPtr dom)
/* HV's earlier than 3.1.0 don't include the HVM flags in guests
status*/
if (hv_versions.hypervisor 2 ||
hv_versions.dom_interface 4) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(unsupported in dom interface 4));
-return NULL;
+return xenDaemonDomainGetOSType(dom);
}
XEN_GETDOMAININFO_CLEAR(dominfo);
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index c759636..75c1514 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1373,15 +1373,11 @@ xenDaemonDomainDestroy(virDomainPtr domain)
* Returns the new string or NULL in case of error, the string must be
* freed by the caller.
*/
-static char *
+char *
xenDaemonDomainGetOSType(virDomainPtr domain)
{
char *type;
struct sexpr *root;
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-
-if (domain-id 0 priv-xendConfigVersion
XEND_CONFIG_VERSION_3_0_4)
-return NULL;
/* can we ask for a subset ? worth it ? */
root = sexpr_get(domain-conn, /xend/domain/%s?detail=1,
domain-name);
@@ -3441,7 +3437,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
}
struct xenUnifiedDriver xenDaemonDriver = {
-.xenDomainGetOSType = xenDaemonDomainGetOSType,
.xenDomainGetMaxMemory = xenDaemonDomainGetMaxMemory,
.xenDomainSetMaxMemory = xenDaemonDomainSetMaxMemory,
.xenDomainSetMemory = xenDaemonDomainSetMemory,
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index d393ec8..9681068 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -108,6 +108,8 @@ char *xenDaemonDomainGetXMLDesc(virDomainPtr domain,
unsigned int flags,
unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain);
char **xenDaemonListDomainsOld(virConnectPtr xend);
+char *xenDaemonDomainGetOSType
Re: [libvirt] [PATCH 16/40] Remove Xen get hostname driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The xenGetHostname entry point in the xenUnifiedDriver table
was unused.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.h | 1 -
1 file changed, 1 deletion(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 3ac2912..16e9743 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -93,7 +93,6 @@ extern int xenRegister (void);
* structure with direct calls in xen_unified.c.
*/
struct xenUnifiedDriver {
-virDrvConnectGetHostname xenGetHostname;
virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
virDrvDomainSetMemory xenDomainSetMemory;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods
Jim Fehlig wrote:
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally invoke the xenHypervisorLookupDomainByID,
xenHypervisorLookupDomainByUUID or xenDaemonLookupByName
for looking up domains. Fallback to xenXMDomainLookupByUUID
and xenXMDomainLookupByName for legacy XenD without inactive
domain support
Do you think there are any Xen installations running such an old xend
toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the
XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 99
+++--
src/xen/xend_internal.c | 89
src/xen/xend_internal.h | 14 ---
src/xen/xs_internal.c | 62 ---
src/xen/xs_internal.h | 2 -
5 files changed, 22 insertions(+), 244 deletions(-)
I spent some time testing this one and didn't notice any problems.
Apparently some time was not enough time. With this patch, I noticed
'virsh undefine dom' failing because the tri-state virDomainIsActive()
is returning -1.
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 82058b7..080045c 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -601,38 +601,17 @@ xenUnifiedDomainCreateXML(virConnectPtr conn,
return xenDaemonCreateXML(conn, xmlDesc);
}
-/* Assumption made in underlying drivers:
- * If the domain is not found and there is no other error, then
- * the Lookup* functions return a NULL but do not set virterror.
- */
static virDomainPtr
xenUnifiedDomainLookupByID(virConnectPtr conn, int id)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-virDomainPtr ret;
+virDomainPtr ret = NULL;
-/* Reset any connection-level errors in virterror first, in case
- * there is one hanging around from a previous call.
- */
-virConnResetLastError(conn);
+ret = xenHypervisorLookupDomainByID(conn, id);
-/* Try hypervisor/xenstore combo. */
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorLookupDomainByID(conn, id);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonLookupByID(conn, id);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
-/* Not found. */
-virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
-return NULL;
+return ret;
}
static virDomainPtr
@@ -642,35 +621,20 @@ xenUnifiedDomainLookupByUUID(virConnectPtr conn,
xenUnifiedPrivatePtr priv = conn-privateData;
virDomainPtr ret;
-/* Reset any connection-level errors in virterror first, in case
- * there is one hanging around from a previous call.
- */
-virConnResetLastError(conn);
-
-/* Try hypervisor/xenstore combo. */
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorLookupDomainByUUID(conn, uuid);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonLookupByUUID(conn, uuid);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
+ret = xenHypervisorLookupDomainByUUID(conn, uuid);
/* Try XM for inactive domains. */
-if (priv-opened[XEN_UNIFIED_XM_OFFSET]) {
-ret = xenXMDomainLookupByUUID(conn, uuid);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
+if (!ret) {
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3)
+ret = xenXMDomainLookupByUUID(conn, uuid);
+else
+return xenDaemonLookupByUUID(conn, uuid);
}
-/* Not found. */
-virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
-return NULL;
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
}
static virDomainPtr
@@ -680,35 +644,16 @@ xenUnifiedDomainLookupByName(virConnectPtr conn,
xenUnifiedPrivatePtr priv = conn-privateData;
virDomainPtr ret;
-/* Reset any connection-level errors in virterror first, in case
- * there is one hanging around from a previous call.
- */
-virConnResetLastError(conn);
-
-/* Try xend. */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonLookupByName(conn, name);
-if (ret || conn-err.code != VIR_ERR_OK)
-return ret;
-}
-
-/* Try xenstore for inactive domains. */
-if (priv-opened[XEN_UNIFIED_XS_OFFSET]) {
-ret
Re: [libvirt] [PATCH 17/40] Simplify the Xen domain get/set (max) memory driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Simplify the Xen memory limit driver methods to directly call
the most appropriate sub-driver
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 50 ++-
src/xen/xen_driver.h | 3 --
src/xen/xen_hypervisor.c | 35 ---
src/xen/xen_hypervisor.h | 3 +-
src/xen/xend_internal.c | 15
src/xen/xm_internal.c| 16 +
src/xen/xs_internal.c| 90
src/xen/xs_internal.h| 4 ---
8 files changed, 36 insertions(+), 180 deletions(-)
Another nice cleanup. I've tested this one as well and didn't notice any
problems. ACK.
I'll have to continue with the reviews tomorrow.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 8ee3c4c..7d09c6b 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -801,53 +801,41 @@ static unsigned long long
xenUnifiedDomainGetMaxMemory(virDomainPtr dom)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-unsigned long long ret;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainGetMaxMemory) {
-ret = drivers[i]-xenDomainGetMaxMemory(dom);
-if (ret != 0) return ret;
-}
-return 0;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainGetMaxMemory(dom);
+else
+return xenDaemonDomainGetMaxMemory(dom);
+} else {
+return xenHypervisorGetMaxMemory(dom);
+}
}
static int
xenUnifiedDomainSetMaxMemory(virDomainPtr dom, unsigned long memory)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-/* Prefer xend for setting max memory */
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-if (xenDaemonDomainSetMaxMemory(dom, memory) == 0)
-return 0;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainSetMaxMemory(dom, memory);
+else
+return xenDaemonDomainSetMaxMemory(dom, memory);
+} else {
+return xenHypervisorSetMaxMemory(dom, memory);
}
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (i != XEN_UNIFIED_XEND_OFFSET
-priv-opened[i]
-drivers[i]-xenDomainSetMaxMemory
-drivers[i]-xenDomainSetMaxMemory(dom, memory) == 0)
-return 0;
-
-return -1;
}
static int
xenUnifiedDomainSetMemory(virDomainPtr dom, unsigned long memory)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenDomainSetMemory
-drivers[i]-xenDomainSetMemory(dom, memory) == 0)
-return 0;
-return -1;
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainSetMemory(dom, memory);
+else
+return xenDaemonDomainSetMemory(dom, memory);
}
static int
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 16e9743..4509161 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -93,9 +93,6 @@ extern int xenRegister (void);
* structure with direct calls in xen_unified.c.
*/
struct xenUnifiedDriver {
-virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
-virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
-virDrvDomainSetMemory xenDomainSetMemory;
virDrvDomainGetInfo xenDomainGetInfo;
virDrvDomainPinVcpu xenDomainPinVcpu;
virDrvDomainGetVcpus xenDomainGetVcpus;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 8636d52..7662843 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -870,11 +870,7 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
# error unsupported platform
#endif
-static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
-
struct xenUnifiedDriver xenHypervisorDriver = {
-.xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
-.xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
.xenDomainGetInfo = xenHypervisorGetDomainInfo,
.xenDomainPinVcpu = xenHypervisorPinVcpu,
.xenDomainGetVcpus = xenHypervisorGetVcpus,
@@ -2763,9 +2759,8 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
/**
- * xenHypervisorGetDomMaxMemory:
- * @conn: connection data
- * @id: domain id
+ * xenHypervisorDomMaxMemory:
+ * @dom: domain
*
* Retrieve the maximum amount of physical memory allocated to a
* domain.
@@ -2773,9 +2768,9 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn
ATTRIBUTE_UNUSED,
* Returns the memory size in kilobytes or 0 in case of error.
*/
unsigned long
Re: [libvirt] [PATCH 18/40] Simplify the Xen domain get info/state driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make the xenUnifiedDomainGetInfo and xenUnifiedDomainGetState drivers
call the correct sub-driver APIs directly.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 52 +++--
src/xen/xen_driver.h | 1 -
src/xen/xen_hypervisor.c | 14 +-
src/xen/xen_hypervisor.h | 3 +-
src/xen/xend_internal.c | 15 +--
src/xen/xend_internal.h | 3 +-
src/xen/xm_internal.c| 15 ++-
src/xen/xm_internal.h| 3 +-
src/xen/xs_internal.c| 115
---
src/xen/xs_internal.h| 9
10 files changed, 23 insertions(+), 207 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 7d09c6b..988dbce 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -84,7 +84,6 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS]
= {
[XEN_UNIFIED_HYPERVISOR_OFFSET] = xenHypervisorDriver,
[XEN_UNIFIED_XEND_OFFSET] = xenDaemonDriver,
-[XEN_UNIFIED_XS_OFFSET] = xenStoreDriver,
[XEN_UNIFIED_XM_OFFSET] = xenXMDriver,
};
The comment for this structure states The five Xen drivers below us.
Now down to three, maybe down to zero by the end of this series :).
ACK.
Regards,
Jim
@@ -842,15 +841,15 @@ static int
xenUnifiedDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenDomainGetInfo
-drivers[i]-xenDomainGetInfo(dom, info) == 0)
-return 0;
-return -1;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainGetInfo(dom, info);
+else
+return xenDaemonDomainGetInfo(dom, info);
+} else {
+return xenHypervisorGetDomainInfo(dom, info);
+}
}
static int
@@ -860,38 +859,17 @@ xenUnifiedDomainGetState(virDomainPtr dom,
unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int ret;
virCheckFlags(0, -1);
-/* trying drivers in the same order as GetInfo for consistent results:
- * hypervisor, xend, xs, and xm */
-
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorGetDomainState(dom, state, reason, flags);
-if (ret = 0)
-return ret;
-}
-
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonDomainGetState(dom, state, reason, flags);
-if (ret = 0)
-return ret;
-}
-
-if (priv-opened[XEN_UNIFIED_XS_OFFSET]) {
-ret = xenStoreDomainGetState(dom, state, reason, flags);
-if (ret = 0)
-return ret;
-}
-
-if (priv-opened[XEN_UNIFIED_XM_OFFSET]) {
-ret = xenXMDomainGetState(dom, state, reason, flags);
-if (ret = 0)
-return ret;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainGetState(dom, state, reason);
+else
+return xenDaemonDomainGetState(dom, state, reason);
+} else {
+return xenHypervisorGetDomainState(dom, state, reason);
}
-
-return -1;
}
static int
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 4509161..953da64 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -93,7 +93,6 @@ extern int xenRegister (void);
* structure with direct calls in xen_unified.c.
*/
struct xenUnifiedDriver {
-virDrvDomainGetInfo xenDomainGetInfo;
virDrvDomainPinVcpu xenDomainPinVcpu;
virDrvDomainGetVcpus xenDomainGetVcpus;
virDrvConnectListDefinedDomains xenListDefinedDomains;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 7662843..5735b0b 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -871,7 +871,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
#endif
struct xenUnifiedDriver xenHypervisorDriver = {
-.xenDomainGetInfo = xenHypervisorGetDomainInfo,
.xenDomainPinVcpu = xenHypervisorPinVcpu,
.xenDomainGetVcpus = xenHypervisorGetVcpus,
.xenDomainGetSchedulerType = xenHypervisorGetSchedulerType,
@@ -2880,11 +2879,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id,
virDomainInfoPtr info)
int
xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
{
-if (domain-id 0)
-return -1;
-
return xenHypervisorGetDomInfo(domain-conn, domain-id, info);
-
}
/**
@@ -2892,7 +2887,6 @@ xenHypervisorGetDomainInfo(virDomainPtr domain,
virDomainInfoPtr info)
* @domain: pointer to the domain block
* @state: returned state of
Re: [libvirt] [PATCH 19/40] Simplify the Xen domain save/restore driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally call the XenD APIs for save/restore, since that
driver will always be open.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 21 -
src/xen/xend_internal.c | 2 ++
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 988dbce..a963e08 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -876,8 +876,6 @@ static int
xenUnifiedDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
unsigned int flags)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-
virCheckFlags(0, -1);
if (dxml) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
@@ -885,9 +883,7 @@ xenUnifiedDomainSaveFlags(virDomainPtr dom, const char
*to, const char *dxml,
return -1;
}
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonDomainSave(dom, to);
-return -1;
+return xenDaemonDomainSave(dom, to);
}
static int
@@ -923,8 +919,7 @@ xenUnifiedDomainManagedSave(virDomainPtr dom, unsigned
int flags)
if (!name)
goto cleanup;
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-ret = xenDaemonDomainSave(dom, name);
+ret = xenDaemonDomainSave(dom, name);
cleanup:
VIR_FREE(name);
@@ -971,8 +966,6 @@ static int
xenUnifiedDomainRestoreFlags(virConnectPtr conn, const char *from,
const char *dxml, unsigned int flags)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-
virCheckFlags(0, -1);
if (dxml) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
@@ -980,9 +973,7 @@ xenUnifiedDomainRestoreFlags(virConnectPtr conn, const
char *from,
return -1;
}
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonDomainRestore(conn, from);
-return -1;
+return xenDaemonDomainRestore(conn, from);
}
static int
@@ -994,11 +985,7 @@ xenUnifiedDomainRestore(virConnectPtr conn, const char
*from)
static int
xenUnifiedDomainCoreDump(virDomainPtr dom, const char *to, unsigned int
flags)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonDomainCoreDump(dom, to, flags);
-return -1;
+return xenDaemonDomainCoreDump(dom, to, flags);
}
static int
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index d6a3698..ccbbf66 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1422,6 +1422,8 @@ xenDaemonDomainSave(virDomainPtr domain, const char
*filename)
/* We can't save the state of Domain-0, that would mean stopping it too
*/
if (domain-id == 0) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(Cannot save host domain));
I'm surprised there have never been complaints about the lack of error
message here.
ACK.
Regards,
Jim
return -1;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 20/40] Simplify the Xen domain VCPU driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 91
src/xen/xen_driver.h | 2 --
src/xen/xen_hypervisor.c | 90 ---
src/xen/xen_hypervisor.h | 3 --
src/xen/xend_internal.c | 25 +
src/xen/xm_internal.c| 6
6 files changed, 38 insertions(+), 179 deletions(-)
Didn't notice any regression in my testing. Looks good.
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index a963e08..1db831d 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -993,7 +993,6 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned
int nvcpus,
unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int ret;
virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
VIR_DOMAIN_VCPU_CONFIG |
@@ -1017,38 +1016,25 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom,
unsigned int nvcpus,
/* Try non-hypervisor methods first, then hypervisor direct method
* as a last resort.
*/
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonDomainSetVcpusFlags(dom, nvcpus, flags);
-if (ret != -2)
-return ret;
-}
-if (priv-opened[XEN_UNIFIED_XM_OFFSET]) {
-ret = xenXMDomainSetVcpusFlags(dom, nvcpus, flags);
-if (ret != -2)
-return ret;
-}
-if (flags == VIR_DOMAIN_VCPU_LIVE)
-return xenHypervisorSetVcpus(dom, nvcpus);
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainSetVcpusFlags(dom, nvcpus, flags);
+else
+return xenDaemonDomainSetVcpusFlags(dom, nvcpus, flags);
}
static int
xenUnifiedDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
{
+xenUnifiedPrivatePtr priv = dom-conn-privateData;
unsigned int flags = VIR_DOMAIN_VCPU_LIVE;
/* Per the documented API, it is hypervisor-dependent whether this
* affects just _LIVE or _LIVE|_CONFIG; in xen's case, that
* depends on xendConfigVersion. */
-if (dom) {
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
-flags |= VIR_DOMAIN_VCPU_CONFIG;
-return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
-}
-return -1;
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
+flags |= VIR_DOMAIN_VCPU_CONFIG;
+
+return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
}
static int
@@ -1056,15 +1042,15 @@ xenUnifiedDomainPinVcpu(virDomainPtr dom, unsigned
int vcpu,
unsigned char *cpumap, int maplen)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenDomainPinVcpu
-drivers[i]-xenDomainPinVcpu(dom, vcpu, cpumap, maplen) == 0)
-return 0;
-return -1;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainPinVcpu(dom, vcpu, cpumap, maplen);
+else
+return xenDaemonDomainPinVcpu(dom, vcpu, cpumap, maplen);
+} else {
+return xenHypervisorPinVcpu(dom, vcpu, cpumap, maplen);
+}
}
static int
@@ -1073,42 +1059,39 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
unsigned char *cpumaps, int maplen)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i, ret;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainGetVcpus) {
-ret = drivers[i]-xenDomainGetVcpus(dom, info, maxinfo, cpumaps,
maplen);
-if (ret 0)
-return ret;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot get VCPUs of inactive domain));
+return -1;
+} else {
+return xenDaemonDomainGetVcpus(dom, info, maxinfo, cpumaps,
maplen);
}
-return -1;
+} else {
+return xenHypervisorGetVcpus(dom, info, maxinfo, cpumaps, maplen);
+}
}
static int
xenUnifiedDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int ret;
virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
VIR_DOMAIN_VCPU_CONFIG |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret =
Re: [libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods
Daniel P. Berrange wrote:
On Wed, May 08, 2013 at 11:37:44AM +0100, Daniel P. Berrange wrote:
On Mon, May 06, 2013 at 09:40:43PM -0600, Jim Fehlig wrote:
Jim Fehlig wrote:
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally invoke the xenHypervisorLookupDomainByID,
xenHypervisorLookupDomainByUUID or xenDaemonLookupByName
for looking up domains. Fallback to xenXMDomainLookupByUUID
and xenXMDomainLookupByName for legacy XenD without inactive
domain support
Do you think there are any Xen installations running such an old xend
toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the
XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 99
+++--
src/xen/xend_internal.c | 89
src/xen/xend_internal.h | 14 ---
src/xen/xs_internal.c | 62 ---
src/xen/xs_internal.h | 2 -
5 files changed, 22 insertions(+), 244 deletions(-)
I spent some time testing this one and didn't notice any problems.
Apparently some time was not enough time. With this patch, I noticed
'virsh undefine dom' failing because the tri-state virDomainIsActive()
is returning -1.
Opps, I made a mistake only checking the hypervisor, which of course
will not know about the domain if it is shutoff :-) Adding the following
extra hunk fixes it
Sigh, helps if i paste the right patch
@@ -660,15 +660,26 @@ xenUnifiedDomainLookupByName(virConnectPtr conn,
static int
xenUnifiedDomainIsActive(virDomainPtr dom)
{
+xenUnifiedPrivatePtr priv = dom-conn-privateData;
virDomainPtr currdom;
int ret = -1;
/* ID field in dom may be outdated, so re-lookup */
currdom = xenHypervisorLookupDomainByUUID(dom-conn, dom-uuid);
+/* Try XM for inactive domains. */
+if (!currdom) {
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3)
+currdom = xenXMDomainLookupByUUID(dom-conn, dom-uuid);
+else
+currdom = xenDaemonLookupByUUID(dom-conn, dom-uuid);
+}
+
if (currdom) {
ret = currdom-id == -1 ? 0 : 1;
virDomainFree(currdom);
+} else if (virGetLastError() == NULL) {
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
}
return ret;
Yep, this fixes the issue. ACK with it squashed in.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods
Daniel P. Berrange wrote: On Mon, May 06, 2013 at 03:01:25PM -0600, Jim Fehlig wrote: Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Unconditionally invoke the xenHypervisorLookupDomainByID, xenHypervisorLookupDomainByUUID or xenDaemonLookupByName for looking up domains. Fallback to xenXMDomainLookupByUUID and xenXMDomainLookupByName for legacy XenD without inactive domain support Do you think there are any Xen installations running such an old xend toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code. This is all relevant to RHEL5 Xen and I'm not ready to declare RHEL5 unsupported by libvirt yet. Understood :). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 21/40] Simplify the Xen domain get XML driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The xenUnifiedDomainGetXMLDesc driver can assume that
the XM and XenD drivers are always present
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 24 +---
src/xen/xend_internal.c | 6 --
2 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 1db831d..f11e5bf 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1106,23 +1106,17 @@ xenUnifiedDomainGetXMLDesc(virDomainPtr dom, unsigned
int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-if (dom-id == -1 priv-xendConfigVersion
XEND_CONFIG_VERSION_3_0_4) {
-if (priv-opened[XEN_UNIFIED_XM_OFFSET])
-return xenXMDomainGetXMLDesc(dom, flags);
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+return xenXMDomainGetXMLDesc(dom, flags);
} else {
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-char *cpus, *res;
-xenUnifiedLock(priv);
-cpus = xenDomainUsedCpus(dom);
-xenUnifiedUnlock(priv);
-res = xenDaemonDomainGetXMLDesc(dom, flags, cpus);
-VIR_FREE(cpus);
-return res;
-}
+char *cpus, *res;
+xenUnifiedLock(priv);
+cpus = xenDomainUsedCpus(dom);
+xenUnifiedUnlock(priv);
+res = xenDaemonDomainGetXMLDesc(dom, flags, cpus);
+VIR_FREE(cpus);
+return res;
}
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return NULL;
}
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index dbad83f..930b882 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1610,17 +1610,11 @@ xenDaemonDomainGetXMLDesc(virDomainPtr domain,
unsigned int flags,
const char *cpus)
{
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
virDomainDefPtr def;
char *xml;
/* Flags checked by virDomainDefFormat */
-if (domain-id 0 priv-xendConfigVersion
XEND_CONFIG_VERSION_3_0_4) {
-/* fall-through to the next driver to handle */
-return NULL;
-}
-
This function has another caller, xenUnifiedDomainMigrateFinish, which
appears to only support xendConfigVersion 3_0_4 anyhow. ACK to the
changes in this patch.
Regards,
Jim
if (!(def = xenDaemonDomainFetch(domain-conn,
domain-id,
domain-name,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 22/40] Simplify the Xen domain migration driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
All the migration code is done by the XenD subdriver which
can be assumed to always be present
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 32 +---
1 file changed, 9 insertions(+), 23 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index f11e5bf..cfdc940 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1233,17 +1233,11 @@ xenUnifiedDomainMigratePrepare(virConnectPtr dconn,
const char *dname,
unsigned long resource)
{
-xenUnifiedPrivatePtr priv = dconn-privateData;
-
virCheckFlags(XEN_MIGRATION_FLAGS, -1);
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonDomainMigratePrepare(dconn, cookie, cookielen,
- uri_in, uri_out,
- flags, dname, resource);
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
+return xenDaemonDomainMigratePrepare(dconn, cookie, cookielen,
+ uri_in, uri_out,
+ flags, dname, resource);
}
static int
@@ -1255,16 +1249,10 @@ xenUnifiedDomainMigratePerform(virDomainPtr dom,
const char *dname,
unsigned long resource)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-
virCheckFlags(XEN_MIGRATION_FLAGS, -1);
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonDomainMigratePerform(dom, cookie, cookielen, uri,
- flags, dname, resource);
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
+return xenDaemonDomainMigratePerform(dom, cookie, cookielen, uri,
+ flags, dname, resource);
}
static virDomainPtr
@@ -1281,24 +1269,22 @@ xenUnifiedDomainMigrateFinish(virConnectPtr dconn,
virCheckFlags(XEN_MIGRATION_FLAGS, NULL);
-dom = xenUnifiedDomainLookupByName(dconn, dname);
-if (! dom) {
+if (!(dom = xenUnifiedDomainLookupByName(dconn, dname)))
return NULL;
-}
if (flags VIR_MIGRATE_PERSIST_DEST) {
domain_xml = xenDaemonDomainGetXMLDesc(dom, 0, NULL);
Ah, my comment in the previous patch referred to the call to
xenDaemonDomainGetXMLDesc here. dom should be inactive at this point
right? In which case xenDaemonDomainGetXMLDesc will return NULL if
xendConfigVersion 3_0_4. But again, not sure if the migration
functions are meant to be supported in that environment.
Patch looks good otherwise and weak ACK assuming the migration functions
are for xendConfigVersion = 3_0_4.
Regards,
Jim
if (! domain_xml) {
virReportError(VIR_ERR_MIGRATE_PERSIST_FAILED,
%s, _(failed to get XML representation of
migrated domain));
-goto failure;
+goto error;
}
dom_new = xenDaemonDomainDefineXML(dconn, domain_xml);
if (! dom_new) {
virReportError(VIR_ERR_MIGRATE_PERSIST_FAILED,
%s, _(failed to define domain on destination
host));
-goto failure;
+goto error;
}
/* Free additional reference added by Define */
@@ -1310,7 +1296,7 @@ xenUnifiedDomainMigrateFinish(virConnectPtr dconn,
return dom;
-failure:
+error:
virDomainFree(dom);
VIR_FREE(domain_xml);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/40] Simplify the Xen driver define domain driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Directly call either XenD or the XM driver for handling
domain define operations.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 28 ++--
src/xen/xen_driver.h| 2 --
src/xen/xend_internal.c | 18 +++---
src/xen/xend_internal.h | 6 ++
src/xen/xm_internal.c | 2 --
5 files changed, 19 insertions(+), 37 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index cfdc940..f504539 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1309,32 +1309,24 @@ xenUnifiedConnectListDefinedDomains(virConnectPtr
conn, char **const names,
int maxnames)
{
xenUnifiedPrivatePtr priv = conn-privateData;
-int i;
-int ret;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenListDefinedDomains) {
-ret = drivers[i]-xenListDefinedDomains(conn, names, maxnames);
-if (ret = 0) return ret;
-}
-return -1;
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+return xenXMListDefinedDomains(conn, names, maxnames);
+} else {
+return xenDaemonListDefinedDomains(conn, names, maxnames);
+}
}
static int
xenUnifiedConnectNumOfDefinedDomains(virConnectPtr conn)
{
xenUnifiedPrivatePtr priv = conn-privateData;
-int i;
-int ret;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenNumOfDefinedDomains) {
-ret = drivers[i]-xenNumOfDefinedDomains(conn);
-if (ret = 0) return ret;
-}
-return -1;
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+return xenXMNumOfDefinedDomains(conn);
+} else {
+return xenDaemonNumOfDefinedDomains(conn);
+}
}
static int
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 4b18b4d..c756dde 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -93,8 +93,6 @@ extern int xenRegister (void);
* structure with direct calls in xen_unified.c.
*/
struct xenUnifiedDriver {
-virDrvConnectListDefinedDomains xenListDefinedDomains;
-virDrvConnectNumOfDefinedDomains xenNumOfDefinedDomains;
virDrvDomainCreate xenDomainCreate;
virDrvDomainDefineXML xenDomainDefineXML;
virDrvDomainUndefine xenDomainUndefine;
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 930b882..addc547 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2938,19 +2938,12 @@ xenDaemonDomainUndefine(virDomainPtr domain)
*
* Returns the number of domain found or -1 in case of error
*/
-static int
+int
xenDaemonNumOfDefinedDomains(virConnectPtr conn)
{
struct sexpr *root = NULL;
int ret = -1;
struct sexpr *_for_i, *node;
-xenUnifiedPrivatePtr priv = conn-privateData;
-
-/* xm_internal.c (the support for defined domains from /etc/xen
- * config files used by old Xen) will handle this.
- */
-if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
-return -1;
root = sexpr_get(conn, /xend/domain?state=halted);
if (root == NULL)
@@ -2971,7 +2964,8 @@ error:
return ret;
}
-static int
+
Extra whitespace.
+int
xenDaemonListDefinedDomains(virConnectPtr conn,
char **const names,
int maxnames)
@@ -2979,10 +2973,6 @@ xenDaemonListDefinedDomains(virConnectPtr conn,
struct sexpr *root = NULL;
int i, ret = -1;
struct sexpr *_for_i, *node;
-xenUnifiedPrivatePtr priv = conn-privateData;
-
-if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
-return -1;
if (maxnames == 0)
return 0;
@@ -3388,8 +3378,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
}
struct xenUnifiedDriver xenDaemonDriver = {
-.xenListDefinedDomains = xenDaemonListDefinedDomains,
-.xenNumOfDefinedDomains = xenDaemonNumOfDefinedDomains,
.xenDomainCreate = xenDaemonDomainCreate,
.xenDomainDefineXML = xenDaemonDomainDefineXML,
.xenDomainUndefine = xenDaemonDomainUndefine,
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index fd661c9..d773ef9 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -109,6 +109,12 @@ char **xenDaemonListDomainsOld(virConnectPtr xend);
char *xenDaemonDomainGetOSType(virDomainPtr domain);
+int xenDaemonNumOfDefinedDomains(virConnectPtr conn);
+int xenDaemonListDefinedDomains(virConnectPtr conn,
+char **const names,
+int maxnames);
+
+
Extra whitespace.
virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr);
int xenDaemonDomainCreate(virDomainPtr domain);
Re: [libvirt] [PATCH 25/40] Simplify the Xen domain define/undefine driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make the domain define/undefine driver methods directly call
into either the XenD or XM drivers
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 24
src/xen/xen_driver.h| 2 --
src/xen/xend_internal.c | 10 --
src/xen/xm_internal.c | 5 -
4 files changed, 8 insertions(+), 33 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index f6c1891..6643a97 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1369,31 +1369,23 @@ static virDomainPtr
xenUnifiedDomainDefineXML(virConnectPtr conn, const char *xml)
{
xenUnifiedPrivatePtr priv = conn-privateData;
-int i;
-virDomainPtr ret;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainDefineXML) {
-ret = drivers[i]-xenDomainDefineXML(conn, xml);
-if (ret) return ret;
-}
-return NULL;
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainDefineXML(conn, xml);
+else
+return xenDaemonDomainDefineXML(conn, xml);
}
static int
xenUnifiedDomainUndefineFlags(virDomainPtr dom, unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
virCheckFlags(0, -1);
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainUndefine
-drivers[i]-xenDomainUndefine(dom) == 0)
-return 0;
-
-return -1;
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainUndefine(dom);
+else
+return xenDaemonDomainUndefine(dom);
}
static int
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index e2c0d68..254c2f5 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -93,8 +93,6 @@ extern int xenRegister (void);
* structure with direct calls in xen_unified.c.
*/
struct xenUnifiedDriver {
-virDrvDomainDefineXML xenDomainDefineXML;
-virDrvDomainUndefine xenDomainUndefine;
virDrvDomainAttachDeviceFlags xenDomainAttachDeviceFlags;
virDrvDomainDetachDeviceFlags xenDomainDetachDeviceFlags;
virDrvDomainGetSchedulerType xenDomainGetSchedulerType;
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index dcd31de..f9b43b8 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2860,9 +2860,6 @@ xenDaemonDomainDefineXML(virConnectPtr conn, const char
*xmlDesc)
xenUnifiedPrivatePtr priv = conn-privateData;
virDomainDefPtr def;
-if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
-return NULL;
-
if (!(def = virDomainDefParseString(xmlDesc, priv-caps, priv-xmlopt,
1 VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE))) {
@@ -2905,11 +2902,6 @@ xenDaemonDomainCreate(virDomainPtr domain)
int
xenDaemonDomainUndefine(virDomainPtr domain)
{
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-
-if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
-return -1;
-
return xend_op(domain-conn, domain-name, op, delete, NULL);
}
@@ -3361,8 +3353,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
}
struct xenUnifiedDriver xenDaemonDriver = {
-.xenDomainDefineXML = xenDaemonDomainDefineXML,
-.xenDomainUndefine = xenDaemonDomainUndefine,
.xenDomainAttachDeviceFlags = xenDaemonAttachDeviceFlags,
.xenDomainDetachDeviceFlags = xenDaemonDetachDeviceFlags,
.xenDomainGetSchedulerType = xenDaemonGetSchedulerType,
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 505f959..bb79c63 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -81,8 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr
domain, const char *xml,
#define XM_XML_ERROR Invalid xml
struct xenUnifiedDriver xenXMDriver = {
-.xenDomainDefineXML = xenXMDomainDefineXML,
-.xenDomainUndefine = xenXMDomainUndefine,
.xenDomainAttachDeviceFlags = xenXMDomainAttachDeviceFlags,
.xenDomainDetachDeviceFlags = xenXMDomainDetachDeviceFlags,
};
@@ -1108,9 +1106,6 @@ xenXMDomainUndefine(virDomainPtr domain)
xenXMConfCachePtr entry;
int ret = -1;
-if (domain-id != -1)
-return -1;
-
xenUnifiedLock(priv);
if (!(filename = virHashLookup(priv-nameConfigMap, domain-name)))
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 24/40] Simplify the Xen domain start driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Directly call either the XenD or XM driver when starting
a persistent domain
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 20 +++-
src/xen/xen_driver.h| 1 -
src/xen/xend_internal.c | 20 +---
src/xen/xm_internal.c | 4
4 files changed, 8 insertions(+), 37 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index f504539..f6c1891 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1333,7 +1333,6 @@ static int
xenUnifiedDomainCreateWithFlags(virDomainPtr dom, unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
int ret = -1;
char *name = NULL;
@@ -1344,21 +1343,16 @@ xenUnifiedDomainCreateWithFlags(virDomainPtr dom,
unsigned int flags)
goto cleanup;
if (virFileExists(name)) {
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-ret = xenDaemonDomainRestore(dom-conn, name);
-if (ret == 0)
-unlink(name);
-}
+ret = xenDaemonDomainRestore(dom-conn, name);
+if (ret == 0)
+unlink(name);
goto cleanup;
}
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i) {
-if (priv-opened[i] drivers[i]-xenDomainCreate
-drivers[i]-xenDomainCreate(dom) == 0) {
-ret = 0;
-goto cleanup;
-}
-}
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+ret = xenXMDomainCreate(dom);
+else
+ret = xenDaemonDomainCreate(dom);
cleanup:
VIR_FREE(name);
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index c756dde..e2c0d68 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -93,7 +93,6 @@ extern int xenRegister (void);
* structure with direct calls in xen_unified.c.
*/
struct xenUnifiedDriver {
-virDrvDomainCreate xenDomainCreate;
virDrvDomainDefineXML xenDomainDefineXML;
virDrvDomainUndefine xenDomainUndefine;
virDrvDomainAttachDeviceFlags xenDomainAttachDeviceFlags;
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index addc547..dcd31de 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2899,24 +2899,7 @@ xenDaemonDomainDefineXML(virConnectPtr conn, const
char *xmlDesc)
int
xenDaemonDomainCreate(virDomainPtr domain)
{
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-int ret;
-virDomainPtr tmp;
-
-if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
-return -1;
-
-ret = xend_op(domain-conn, domain-name, op, start, NULL);
-
-if (ret != -1) {
-/* Need to force a refresh of this object's ID */
-tmp = virDomainLookupByName(domain-conn, domain-name);
-if (tmp) {
-domain-id = tmp-id;
-virDomainFree(tmp);
-}
-}
Should this bit about updating the domain id be removed? Will the dom id
remain at -1? If the id needs updated, it should probably be retrieved
directly from xend.
Regards,
Jim
-return ret;
+return xend_op(domain-conn, domain-name, op, start, NULL);
}
int
@@ -3378,7 +3361,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
}
struct xenUnifiedDriver xenDaemonDriver = {
-.xenDomainCreate = xenDaemonDomainCreate,
.xenDomainDefineXML = xenDaemonDomainDefineXML,
.xenDomainUndefine = xenDaemonDomainUndefine,
.xenDomainAttachDeviceFlags = xenDaemonAttachDeviceFlags,
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index f5348bd..505f959 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -81,7 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr
domain, const char *xml,
#define XM_XML_ERROR Invalid xml
struct xenUnifiedDriver xenXMDriver = {
-.xenDomainCreate = xenXMDomainCreate,
.xenDomainDefineXML = xenXMDomainDefineXML,
.xenDomainUndefine = xenXMDomainUndefine,
.xenDomainAttachDeviceFlags = xenXMDomainAttachDeviceFlags,
@@ -923,9 +922,6 @@ xenXMDomainCreate(virDomainPtr domain)
const char *filename;
xenXMConfCachePtr entry;
-if (domain-id != -1)
-return -1;
-
xenUnifiedLock(priv);
if (!(filename = virHashLookup(priv-nameConfigMap, domain-name)))
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 26/40] Simplify the Xen domain attach/dettach driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make the domain attach/dettach driver methods directly call
into either the XenD or XM drivers
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 55
+
src/xen/xen_driver.h| 2 --
src/xen/xend_internal.c | 27 ++--
src/xen/xend_internal.h | 6 ++
src/xen/xm_internal.c | 13 ++--
src/xen/xm_internal.h | 10 +++--
6 files changed, 37 insertions(+), 76 deletions(-)
ACK.
Regards,
Jim
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 6643a97..2262713 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -84,7 +84,6 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS]
= {
[XEN_UNIFIED_HYPERVISOR_OFFSET] = xenHypervisorDriver,
[XEN_UNIFIED_XEND_OFFSET] = xenDaemonDriver,
-[XEN_UNIFIED_XM_OFFSET] = xenXMDriver,
};
static bool inside_daemon = false;
@@ -1397,7 +1396,6 @@ static int
xenUnifiedDomainAttachDevice(virDomainPtr dom, const char *xml)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE;
/*
@@ -1405,14 +1403,13 @@ xenUnifiedDomainAttachDevice(virDomainPtr dom, const
char *xml)
* config without touching persistent config, we add the extra flag here
* to make this API work
*/
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]
-priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainAttachDeviceFlags
-drivers[i]-xenDomainAttachDeviceFlags(dom, xml, flags) == 0)
-return 0;
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainAttachDeviceFlags(dom, xml, flags);
+else
+return xenDaemonAttachDeviceFlags(dom, xml, flags);
return -1;
}
@@ -1422,21 +1419,17 @@ xenUnifiedDomainAttachDeviceFlags(virDomainPtr dom,
const char *xml,
unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainAttachDeviceFlags
-drivers[i]-xenDomainAttachDeviceFlags(dom, xml, flags) == 0)
-return 0;
-
-return -1;
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainAttachDeviceFlags(dom, xml, flags);
+else
+return xenDaemonAttachDeviceFlags(dom, xml, flags);
}
static int
xenUnifiedDomainDetachDevice(virDomainPtr dom, const char *xml)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE;
/*
@@ -1444,16 +1437,13 @@ xenUnifiedDomainDetachDevice(virDomainPtr dom, const
char *xml)
* config without touching persistent config, we add the extra flag here
* to make this API work
*/
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]
-priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainDetachDeviceFlags
-drivers[i]-xenDomainDetachDeviceFlags(dom, xml, flags) == 0)
-return 0;
-
-return -1;
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainDetachDeviceFlags(dom, xml, flags);
+else
+return xenDaemonDetachDeviceFlags(dom, xml, flags);
}
static int
@@ -1461,25 +1451,18 @@ xenUnifiedDomainDetachDeviceFlags(virDomainPtr dom,
const char *xml,
unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainDetachDeviceFlags
-drivers[i]-xenDomainDetachDeviceFlags(dom, xml, flags) == 0)
-return 0;
-return -1;
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainDetachDeviceFlags(dom, xml, flags);
+else
+return xenDaemonDetachDeviceFlags(dom, xml, flags);
}
static int
xenUnifiedDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
unsigned int flags)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return
Re: [libvirt] [PATCH 27/40] Simplify the Xen domain scheduler parameter driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make the Xen domain scheduler parameter methods directly
call into XenD or Xen hypervisor drivers
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 54
+---
src/xen/xen_driver.h | 15 --
src/xen/xen_hypervisor.c | 25 --
src/xen/xen_hypervisor.h | 1 -
src/xen/xend_internal.c | 12 +++
src/xen/xend_internal.h | 12 +++
6 files changed, 35 insertions(+), 84 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 2262713..ac61677 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -80,12 +80,6 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
unsigned char *cpumaps, int maplen);
-/* The five Xen drivers below us. */
-static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS]
= {
-[XEN_UNIFIED_HYPERVISOR_OFFSET] = xenHypervisorDriver,
-[XEN_UNIFIED_XEND_OFFSET] = xenDaemonDriver,
-};
-
Ah, so my hypothesis in patch 18 was correct, we are down to zero now :).
ACK.
Regards,
Jim
static bool inside_daemon = false;
/**
@@ -1503,17 +1497,17 @@ static char *
xenUnifiedDomainGetSchedulerType(virDomainPtr dom, int *nparams)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-char *schedulertype;
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; i++) {
-if (priv-opened[i] drivers[i]-xenDomainGetSchedulerType) {
-schedulertype = drivers[i]-xenDomainGetSchedulerType(dom,
nparams);
-if (schedulertype != NULL)
-return schedulertype;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot change scheduler parameters));
+return NULL;
}
+return xenDaemonGetSchedulerType(dom, nparams);
+} else {
+return xenHypervisorGetSchedulerType(dom, nparams);
}
-return NULL;
}
static int
@@ -1523,18 +1517,19 @@
xenUnifiedDomainGetSchedulerParametersFlags(virDomainPtr dom,
unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i, ret;
virCheckFlags(0, -1);
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i) {
-if (priv-opened[i] drivers[i]-xenDomainGetSchedulerParameters) {
- ret = drivers[i]-xenDomainGetSchedulerParameters(dom, params,
nparams);
- if (ret == 0)
- return 0;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot change scheduler parameters));
+return -1;
}
+return xenDaemonGetSchedulerParameters(dom, params, nparams);
+} else {
+return xenHypervisorGetSchedulerParameters(dom, params, nparams);
}
-return -1;
}
static int
@@ -1553,20 +1548,19 @@
xenUnifiedDomainSetSchedulerParametersFlags(virDomainPtr dom,
unsigned int flags)
{
xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i, ret;
virCheckFlags(0, -1);
-/* do the hypervisor call last to get better error */
-for (i = XEN_UNIFIED_NR_DRIVERS - 1; i = 0; i--) {
-if (priv-opened[i] drivers[i]-xenDomainSetSchedulerParameters) {
- ret = drivers[i]-xenDomainSetSchedulerParameters(dom, params,
nparams);
- if (ret == 0)
- return 0;
+if (dom-id 0) {
+if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot change scheduler parameters));
+return -1;
}
+return xenDaemonSetSchedulerParameters(dom, params, nparams);
+} else {
+return xenHypervisorSetSchedulerParameters(dom, params, nparams);
}
-
-return -1;
}
static int
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index e8c2958..e33610d 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -82,21 +82,6 @@ extern int xenRegister (void);
VIR_MIGRATE_PAUSED | \
VIR_MIGRATE_PERSIST_DEST)
-/* _xenUnifiedDriver:
- *
- * Entry points into the underlying Xen drivers. This structure
- * will eventually go away and instead xen unified will make direct
- * calls to the underlying Xen drivers.
- *
- * To reiterate - the goal is to remove elements from this structure
- * until it is empty, replacing indirect calls through this
- * structure with direct calls in xen_unified.c.
- */
-struct xenUnifiedDriver {
-
Re: [libvirt] [PATCH 28/40] Simplify the Xen domain autostart driver method
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Unconditionally call into the XenD or XM drivers for autostart
handling, since they are guaranteed to be open
---
src/xen/xen_driver.c| 18 --
src/xen/xend_internal.c | 14 --
2 files changed, 4 insertions(+), 28 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index ac61677..1941dbe 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1465,15 +1465,10 @@ xenUnifiedDomainGetAutostart(virDomainPtr dom, int
*autostart)
xenUnifiedPrivatePtr priv = dom-conn-privateData;
if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
-if (priv-opened[XEN_UNIFIED_XM_OFFSET])
-return xenXMDomainGetAutostart(dom, autostart);
+return xenXMDomainGetAutostart(dom, autostart);
} else {
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonDomainGetAutostart(dom, autostart);
+return xenDaemonDomainGetAutostart(dom, autostart);
}
Braces no longer needed with the single statement in the 'if' and 'else'.
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
}
static int
@@ -1482,15 +1477,10 @@ xenUnifiedDomainSetAutostart(virDomainPtr dom, int
autostart)
xenUnifiedPrivatePtr priv = dom-conn-privateData;
if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4) {
-if (priv-opened[XEN_UNIFIED_XM_OFFSET])
-return xenXMDomainSetAutostart(dom, autostart);
+return xenXMDomainSetAutostart(dom, autostart);
} else {
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
-return xenDaemonDomainSetAutostart(dom, autostart);
+return xenDaemonDomainSetAutostart(dom, autostart);
}
Same here.
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
}
static char *
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 87b8875..ad69b47 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2541,13 +2541,6 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, int
*autostart)
{
struct sexpr *root;
const char *tmp;
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-
-/* xm_internal.c (the support for defined domains from /etc/xen
- * config files used by old Xen) will handle this.
- */
-if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
-return -1;
root = sexpr_get(domain-conn, /xend/domain/%s?detail=1, domain-name);
if (root == NULL) {
@@ -2574,13 +2567,6 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, int
autostart)
virBuffer buffer = VIR_BUFFER_INITIALIZER;
char *content = NULL;
int ret = -1;
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-
-/* xm_internal.c (the support for defined domains from /etc/xen
- * config files used by old Xen) will handle this.
- */
-if (priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
-return -1;
root = sexpr_get(domain-conn, /xend/domain/%s?detail=1, domain-name);
if (root == NULL) {
ACK.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 29/40] Simplify the Xen domain stats/peek / node memory driver methods
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make the Xen domain stats / peek and node memory driver
methods unconditionally call the sub-drivers which are
guaranteed to be open.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 59
+
src/xen/xend_internal.c | 3 ---
2 files changed, 11 insertions(+), 51 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 1941dbe..d6817eb 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1566,26 +1566,14 @@ static int
xenUnifiedDomainBlockStats(virDomainPtr dom, const char *path,
struct _virDomainBlockStats *stats)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET])
-return xenHypervisorDomainBlockStats(dom, path, stats);
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
+return xenHypervisorDomainBlockStats(dom, path, stats);
}
static int
xenUnifiedDomainInterfaceStats(virDomainPtr dom, const char *path,
struct _virDomainInterfaceStats *stats)
{
-xenUnifiedPrivatePtr priv = dom-conn-privateData;
-
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET])
-return xenHypervisorDomainInterfaceStats(dom, path, stats);
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
+return xenHypervisorDomainInterfaceStats(dom, path, stats);
}
static int
@@ -1593,57 +1581,32 @@ xenUnifiedDomainBlockPeek(virDomainPtr dom, const
char *path,
unsigned long long offset, size_t size,
void *buffer, unsigned int flags)
{
-int r;
xenUnifiedPrivatePtr priv = dom-conn-privateData;
virCheckFlags(0, -1);
-if (priv-opened[XEN_UNIFIED_XEND_OFFSET]) {
-r = xenDaemonDomainBlockPeek(dom, path, offset, size, buffer);
-if (r != -2) return r;
-/* r == -2 means declined, so fall through to XM driver ... */
-}
Heh, hack to make the unified driver keep trying. We won't miss this
code. BTW, it would be good to remove the -2 if declined comment in
the function description. I didn't look for these now outdated comments
in your previous patches where similar changes were made.
ACK.
Regards,
Jim
-
-if (priv-opened[XEN_UNIFIED_XM_OFFSET]) {
-if (xenXMDomainBlockPeek(dom, path, offset, size, buffer) == 0)
-return 0;
-}
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
+if (dom-id 0 priv-xendConfigVersion XEND_CONFIG_VERSION_3_0_4)
+return xenXMDomainBlockPeek(dom, path, offset, size, buffer);
+else
+return xenDaemonDomainBlockPeek(dom, path, offset, size, buffer);
}
static int
xenUnifiedNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long
*freeMems,
int startCell, int maxCells)
{
-xenUnifiedPrivatePtr priv = conn-privateData;
-
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET])
-return xenHypervisorNodeGetCellsFreeMemory(conn, freeMems,
- startCell, maxCells);
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return -1;
+return xenHypervisorNodeGetCellsFreeMemory(conn, freeMems,
+ startCell, maxCells);
}
static unsigned long long
xenUnifiedNodeGetFreeMemory(virConnectPtr conn)
{
unsigned long long freeMem = 0;
-int ret;
-xenUnifiedPrivatePtr priv = conn-privateData;
-if (priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
-ret = xenHypervisorNodeGetCellsFreeMemory(conn, freeMem,
- -1, 1);
-if (ret != 1)
-return 0;
-return freeMem;
-}
-
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return 0;
+if (xenHypervisorNodeGetCellsFreeMemory(conn, freeMem, -1, 1) 0)
+return 0;
+return freeMem;
}
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index ad69b47..ba4a018 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3249,9 +3249,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
int vncport;
const char *actual;
-if (domain-id 0 priv-xendConfigVersion
XEND_CONFIG_VERSION_3_0_4)
-return -2; /* Decline, allow XM to handle it. */
-
/* Security check: The path must correspond to a block device. */
if (domain-id 0)
root = sexpr_get(domain-conn, /xend/domain/%d?detail=1,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 30/40] Convert Xen domain lookup driver methods to use virDomainDefPtr
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Introduce use of a virDomainDefPtr in the domain lookup
APIs to simplify introduction of ACL security checks.
The virDomainPtr cannot be safely used, since the app
may have supplied mis-matching name/uuid/id fields. eg
the name points to domain X, while the uuid points to
domain Y. Resolving the virDomainPtr to a virDomainDefPtr
ensures a consistent name/uuid/id set.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/domain_conf.c | 23 +
src/conf/domain_conf.h | 4 ++
src/libvirt_private.syms | 1 +
src/xen/xen_driver.c | 132
+--
src/xen/xen_hypervisor.c | 17 +++---
src/xen/xen_hypervisor.h | 8 +--
src/xen/xen_inotify.c| 14 ++---
src/xen/xend_internal.c | 28 --
src/xen/xend_internal.h | 4 +-
src/xen/xm_internal.c| 30 ---
src/xen/xm_internal.h| 5 +-
11 files changed, 165 insertions(+), 101 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d945b74..1fef6db 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2048,6 +2048,29 @@ error:
return NULL;
}
+
Extra newline?
+virDomainDefPtr virDomainDefNew(const char *name,
+const unsigned char *uuid,
+int id)
+{
+virDomainDefPtr def;
+
+if (VIR_ALLOC(def) 0) {
+virReportOOMError();
+return NULL;
+}
+
+if (!(def-name = strdup(name))) {
+VIR_FREE(def);
+return NULL;
+}
+
+memcpy(def-uuid, uuid, VIR_UUID_BUFLEN);
+def-id = id;
+
+return def;
+}
+
void virDomainObjAssignDef(virDomainObjPtr domain,
const virDomainDefPtr def,
bool live,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3a0f23a..d5d05f7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2148,6 +2148,10 @@ void virDomainDefFree(virDomainDefPtr vm);
virDomainChrDefPtr virDomainChrDefNew(void);
+virDomainDefPtr virDomainDefNew(const char *name,
+const unsigned char *uuid,
+int id);
+
enum {
VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 0),
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE = (1 1),
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 89b65b5..3b625a8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -117,6 +117,7 @@ virDomainDefGenSecurityLabelDef;
virDomainDefGetDefaultEmulator;
virDomainDefGetSecurityLabelDef;
virDomainDefMaybeAddController;
+virDomainDefNew;
virDomainDefParseFile;
virDomainDefParseNode;
virDomainDefParseString;
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index d6817eb..86df4b6 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -82,6 +82,58 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
static bool inside_daemon = false;
+static virDomainDefPtr xenGetDomainDefForID(virConnectPtr conn, int id)
+{
+virDomainDefPtr ret;
+
+ret = xenHypervisorLookupDomainByID(conn, id);
+
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
+}
+
+static virDomainDefPtr xenGetDomainDefForName(virConnectPtr conn, const char
*name)
+{
+xenUnifiedPrivatePtr priv = conn-privateData;
+virDomainDefPtr ret;
+
+ret = xenDaemonLookupByName(conn, name);
+
+/* Try XM for inactive domains. */
+if (!ret
+priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3)
+ret = xenXMDomainLookupByName(conn, name);
+
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
+}
+
+static virDomainDefPtr xenGetDomainDefForUUID(virConnectPtr conn, const
unsigned char *uuid)
+{
+xenUnifiedPrivatePtr priv = conn-privateData;
+virDomainDefPtr ret;
+
+ret = xenHypervisorLookupDomainByUUID(conn, uuid);
+
+/* Try XM for inactive domains. */
+if (!ret) {
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3)
+ret = xenXMDomainLookupByUUID(conn, uuid);
+else
+ret = xenDaemonLookupByUUID(conn, uuid);
+}
+
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
+}
+
+
Extra newline.
/**
* xenNumaInit:
* @conn: pointer to the hypervisor connection
@@ -597,12 +649,18 @@ static virDomainPtr
xenUnifiedDomainLookupByID(virConnectPtr conn, int id)
{
virDomainPtr ret = NULL;
+virDomainDefPtr def = NULL;
-ret = xenHypervisorLookupDomainByID(conn, id);
+if (!(def = xenGetDomainDefForID(conn, id)))
+goto cleanup;
-if (!ret virGetLastError()
Re: [libvirt] [PATCH 29/40] Simplify the Xen domain stats/peek / node memory driver methods
Daniel P. Berrange wrote: Thanks for all the reviews so far ! I'm going to push the first 29 patches now. I've tested basic operations and things seem to be working sanely to me. Nice. I've done a fair bit of testing with the first 30 patches applied (including the tweeks to 10 and 24) and haven't noticed any problems either. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix build with old polkit0
Commit 979e9c56 missed one case of providing the timestamp
parameter to virNetServerClientGetUNIXIdentity() when WITH_POLKIT0
is defined.
---
Pushed under the build breaker rule.
daemon/remote.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 3b6446d..1d21478 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2899,6 +2899,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server
ATTRIBUTE_UNUSED,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
DBusConnection *sysbus;
+unsigned long long timestamp;
virMutexLock(priv-lock);
@@ -2913,7 +2914,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server
ATTRIBUTE_UNUSED,
}
if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid,
- callerPid) 0) {
+ callerPid, timestamp) 0) {
VIR_ERROR(_(cannot get peer socket identity));
goto authfail;
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/11] Convert Xen domain lookup driver methods to use virDomainDefPtr
Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Introduce use of a virDomainDefPtr in the domain lookup
APIs to simplify introduction of ACL security checks.
The virDomainPtr cannot be safely used, since the app
may have supplied mis-matching name/uuid/id fields. eg
the name points to domain X, while the uuid points to
domain Y. Resolving the virDomainPtr to a virDomainDefPtr
ensures a consistent name/uuid/id set.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/domain_conf.c | 24
src/conf/domain_conf.h | 4 ++
src/libvirt_private.syms | 1 +
src/xen/xen_driver.c | 147
+++
src/xen/xen_hypervisor.c | 17 +++---
src/xen/xen_hypervisor.h | 8 +--
src/xen/xen_inotify.c| 14 ++---
src/xen/xend_internal.c | 34 +--
src/xen/xend_internal.h | 4 +-
src/xen/xm_internal.c| 30 --
src/xen/xm_internal.h| 5 +-
11 files changed, 173 insertions(+), 115 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d55ce6b..61995cd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2048,6 +2048,30 @@ error:
return NULL;
}
+
Extra newline? I've noticed inconsistencies throughout most of the files
wrt 1 or 2 newlines between function definitions, so difficult to say
which is preferred.
+virDomainDefPtr virDomainDefNew(const char *name,
+const unsigned char *uuid,
+int id)
+{
+virDomainDefPtr def;
+
+if (VIR_ALLOC(def) 0) {
+virReportOOMError();
+return NULL;
+}
+
+if (!(def-name = strdup(name))) {
+VIR_FREE(def);
+return NULL;
+}
+
+memcpy(def-uuid, uuid, VIR_UUID_BUFLEN);
+def-id = id;
+
+return def;
+}
+
+
void virDomainObjAssignDef(virDomainObjPtr domain,
const virDomainDefPtr def,
bool live,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 21f7ce2..f7644a6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2147,6 +2147,10 @@ void virDomainDefFree(virDomainDefPtr vm);
virDomainChrDefPtr virDomainChrDefNew(void);
+virDomainDefPtr virDomainDefNew(const char *name,
+const unsigned char *uuid,
+int id);
+
enum {
VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 0),
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE = (1 1),
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bb70595..d2f5827 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -117,6 +117,7 @@ virDomainDefGenSecurityLabelDef;
virDomainDefGetDefaultEmulator;
virDomainDefGetSecurityLabelDef;
virDomainDefMaybeAddController;
+virDomainDefNew;
virDomainDefParseFile;
virDomainDefParseNode;
virDomainDefParseString;
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index cc54f7a..d9420d8 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -82,6 +82,60 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
static bool is_privileged = false;
+static virDomainDefPtr xenGetDomainDefForID(virConnectPtr conn, int id)
+{
+virDomainDefPtr ret;
+
+ret = xenHypervisorLookupDomainByID(conn, id);
+
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
+}
+
+
+static virDomainDefPtr xenGetDomainDefForName(virConnectPtr conn, const char
*name)
+{
+xenUnifiedPrivatePtr priv = conn-privateData;
+virDomainDefPtr ret;
+
+ret = xenDaemonLookupByName(conn, name);
+
+/* Try XM for inactive domains. */
+if (!ret
+priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3)
+ret = xenXMDomainLookupByName(conn, name);
+
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
+}
+
+
+static virDomainDefPtr xenGetDomainDefForUUID(virConnectPtr conn, const
unsigned char *uuid)
+{
+xenUnifiedPrivatePtr priv = conn-privateData;
+virDomainDefPtr ret;
+
+ret = xenHypervisorLookupDomainByUUID(conn, uuid);
+
+/* Try XM for inactive domains. */
+if (!ret) {
+if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_3)
+ret = xenXMDomainLookupByUUID(conn, uuid);
+else
+ret = xenDaemonLookupByUUID(conn, uuid);
+}
+
+if (!ret virGetLastError() == NULL)
+virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+return ret;
+}
+
+
/**
* xenNumaInit:
* @conn: pointer to the hypervisor connection
@@ -597,12 +651,18 @@ static virDomainPtr
xenUnifiedDomainLookupByID(virConnectPtr conn, int id)
{
virDomainPtr ret = NULL;
+virDomainDefPtr def = NULL;
-ret =
