Re: [libvirt] [PATCH v3 1/4] conf: add xen type for channels
On 09/26/2016 11:33 AM, Joao Martins wrote: > So far only guestfwd and virtio were supported. Add an additional > for Xen as libxl channels create Xen console visible to the guest. > > Signed-off-by: Joao Martins > --- > Changes since v2: > * Add relevant documentation about target type xen. > --- > docs/formatdomain.html.in | 10 ++ > docs/schemas/domaincommon.rng | 11 +++ > src/conf/domain_conf.c| 18 ++ > src/conf/domain_conf.h| 1 + > src/qemu/qemu_command.c | 1 + > 5 files changed, 37 insertions(+), 4 deletions(-) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index f48a4d8..129ba62 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -5980,6 +5980,16 @@ qemu-kvm -net nic,model=? /dev/null > Possible values for the state attribute are > connected and disconnected. > > + xen > + Paravirtualized xen channel. Channel is exposed in the guest as a > +xen console but identified with a name. The setup of the channel > +depends to guest own rules and can live in a arbitrary path (for more > +info, please see href="http://xenbits.xen.org/docs/unstable/misc/channel.txt";>http://xenbits.xen.org/docs/unstable/misc/channel.txt). The last sentence is not clear IMO and I'd like to improve it before pushing this series. What do you think of the below diff? Or feel free to propose something better :-). diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 129ba62..7008005 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5981,12 +5981,12 @@ qemu-kvm -net nic,model=? /dev/null connected and disconnected. xen - Paravirtualized xen channel. Channel is exposed in the guest as a -xen console but identified with a name. The setup of the channel -depends to guest own rules and can live in a arbitrary path (for more -info, please see http://xenbits.xen.org/docs/unstable/misc/channel.txt";>http://xenbits.xen.org/docs/unstable/misc/channel.txt). + Paravirtualized Xen channel. Channel is exposed in the guest as a +Xen console but identified with a name. Setup and consumption of a Xen +channel depends on software and configuration in the guest +(for more info, please see http://xenbits.xen.org/docs/unstable/misc/channel.txt";>http://xenbits.xen.org/docs/unstable/misc/channel.txt). Channel source path semantics are the same as the virtio target type. -Although state attribute is not provided as xen channels +The state attribute is not supported since Xen channels lack the necessary probing mechanism. Since 2.3.0 Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] libxl: channels support
On 09/26/2016 04:48 PM, Joao Martins wrote:
> On 09/26/2016 10:44 PM, Jim Fehlig wrote:
>> On 09/26/2016 11:33 AM, Joao Martins wrote:
>>> And allow libxl to handle channel element which creates a Xen
>>> console visible to the guest as a low-bandwitdh communication
>>> channel. If type is PTY we also fetch the tty after boot using
>>> libxl_channel_getinfo to fetch the tty path. On socket case,
>>> we autogenerate a path if not specified in the XML. Path autogenerated
>>> is slightly different from qemu driver: qemu stores also on
>>> "channels/target" but it creates then a directory per domain with
>>> each channel target name. libxl doesn't appear to have a clear
>>> definition of private files associated with each domain, so for
>>> simplicity we do it slightly different. On qemu each autogenerated
>>> channel goes like:
>>>
>>> channels/target//
>>>
>>> Whereas for libxl:
>>>
>>> channels/target/-
>>>
>>> Should note that if path is not specified it won't persist,
>>> existing only on live XML, unless user had initially specified it.
>>> Since support for libxl channels only came on Xen >= 4.5 we therefore
>>> need to conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
>>>
>>> After this patch and having a qemu guest agent:
>>> $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
>>>
>>>
>>>
>>>
>>>
>>> $ virsh create domain.xml
>>> $ echo '{"execute":"guest-network-get-interfaces"}' | socat
>>> stdio,ignoreeof unix-connect:/tmp/channel
>>>
>>> {"execute":"guest-network-get-interfaces"}
>>> {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
>>> "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
>>> "ip-address": "::1", "prefix": 128}], "hardware-address":
>>> "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
>>> [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
>>> {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
>>> "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
>>> "sit0"}]}
>>>
>>> Signed-off-by: Joao Martins
>>> ---
>>> Since v2:
>>> * Remove ret variable and do similar to other make functions.
>>> * Have channelDir passed as an argument instead of driver.
>>>
>>> Since v1:
>>> * Autogenerated paths, and updated commit message explaning it the
>>> different
>>> naming. Despite per domain name being slightly different, parent
>>> directory is same across both drivers.
>>> ---
>>> src/libxl/libxl_conf.c | 110
>>> +++
>>> src/libxl/libxl_conf.h | 3 ++
>>> src/libxl/libxl_domain.c | 43 +-
>>> src/libxl/libxl_driver.c | 7 +++
>>> 4 files changed, 162 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>>> index 92faa44..4c94d54 100644
>>> --- a/src/libxl/libxl_conf.c
>>> +++ b/src/libxl/libxl_conf.c
>>> @@ -88,6 +88,7 @@ libxlDriverConfigDispose(void *obj)
>>> VIR_FREE(cfg->saveDir);
>>> VIR_FREE(cfg->autoDumpDir);
>>> VIR_FREE(cfg->lockManagerName);
>>> +VIR_FREE(cfg->channelDir);
>>> virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
>>> }
>>>
>>> @@ -1348,6 +1349,8 @@ libxlDriverConfigNew(void)
>>> goto error;
>>> if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0)
>>> goto error;
>>> +if (VIR_STRDUP(cfg->channelDir, LIBXL_CHANNEL_DIR) < 0)
>>> +goto error;
>>>
>>> if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) < 0)
>>> goto error;
>>> @@ -1499,6 +1502,107 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr
>>> cfg,
>>>
>>> }
>>>
>>> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
>>> +static int
>>> +libxlPrepareChannel(virDomainChrDefPtr channel,
>>> +const char *channelDir,
>>> +const char *domainName)
>>> +{
>>> +if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
>>> +channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>>> +!channel->source.data.nix.path) {
>>> +if (virAsprintf(&channel->source.data.nix.path,
>>> +"%s/%s-%s", channelDir, domainName,
>>> +channel->target.name ? channel->target.name
>>> +: "unknown.sock") < 0)
>>> +return -1;
>>> +
>>> +channel->source.data.nix.listen = true;
>>> +}
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static int
>>> +libxlMakeChannel(virDomainChrDefPtr l_channel,
>>> + libxl_device_channel *x_channel)
>>> +{
>>> +libxl_device_channel_init(x_channel);
>>> +
>>> +if (l_channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) {
>>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> + _("channel target type not supported"));
>>> +return -1;
>>> +}
>>> +
>>> +switch (l_channel->source.type) {
>>> +case VIR_DOMAIN_CHR_TYPE_PTY:
>>> +x_cha
Re: [libvirt] [PATCH] network: Add support for configuring dhcp lease time
On Sun, Sep 25, 2016 at 12:58 AM, Laine Stump wrote: > On 09/24/2016 10:51 AM, Nehal J Wani wrote: >> >> On Sat, Sep 24, 2016 at 3:01 AM, Laine Stump wrote: >>> >>> On 09/23/2016 02:43 PM, Martin Wilck wrote: On Fri, 2016-09-23 at 11:56 -0400, Laine Stump wrote: > Martin's patch tries to solve the problem with "dhcp-authoritative" > which, as far as I understand, tells dnsmasq "you are the keeper of > *all* lease information on this network, so if you think the address > is > unused, it really is unused"; dnsmasq uses this information to > freely > grant a guest any address it asks for, as long as there is no > current > lease for it. This sounds troublesome to me - If client A's lease > has > expired (because it's turned off for a bit), is it okay for client B > to > come barging in and insist on grabbing the address that client A has > just lost even though there are many other addresses still > available? > Sure, technically it's legal, but it seems unnecessarily disruptive. I am not sure what you mean. What scenario would be "disrupted" by this approach? >>> >>> >>> Disruptive may not have been the best word. What I mean is that an >>> unfriendly guest with proper information could take advantage of this >>> "give >>> any currently-unused address to anyone who asks for it" policy by >>> purposefully taking over an IP address that it knew had previously been >>> leased to a different guest (and who was expecting on getting the address >>> back the next time it booted). Of course 1) it would take a lot of inside >>> knowledge / good timing / luck to make this happen, and 2) you can argue >>> that any guest who relied on a dhcp address remaining unchanged deserves >>> what it gets (and the same for any other machine trying to contact that >>> guest via the IP address). >>> Without dhcp_authoritative, client A will not get its lease back, >>> >>> >>> Is that true even if dnsmasq isn't restarted, and even if leasefile-ro >>> isn't >>> used? The Openstack discussion on this topic implies that the behavior >>> only >>> exists because they aren't saving a leasefile, not just because the lease >>> is >>> expired. >>> whether or not B applied for it in the meantime, and B wouldn't get the lease, either. The user wouldn't be able to ping either one. If dhcp_authoritative is used, at least one of them will get what it needs (and serving both is impossible - it's hard to tell what's the Right Thing in the situation you describe!). >>> >>> >>> Well, I guess it's a policy decision, but I would tend toward *not* >>> allowing >>> B to demand a particular IP address that it has never before been given. >>> Anyway, unless the virtual network is really crowded (in which case it might make sense for the admin to use a class B network instead) the probability of such clashes should be rather minimal in the real world. It's much more likely that A itself tries to reacquire the lease, and that situation is fixed by dhcp_authoritative. >>> >>> >>> >>> Yeah, as long as the guests are all friendly. My concern is with >>> unfriendly >>> guests. >>> >>> > I think the *real* solution is to fix the lease handling so that > dnsmasq > remembers leases after they've expired (assuming that can be done > once > leasefile-ro is set). They would be marked as "expired", and > probably > not even reported externally, but all their info would still be > there > internally for dnsmasq's use when considering what to do with > incoming > requests for specific IP addresses. Is that possible at all with the current leasehelper setup? >>> >>> >>> "maybe"? See Nehal's recent email - we weren't notifying dnsmasq of >>> expired >>> leases, and that could be the source of the problem (or not, I didn't >>> understand if he's verified whether doing so actually causes dnsmasq to >>> accept the requests for expired addresses). If that works, then the >>> problem >>> is solved. If not, then we may need to consider using dhcp-authoritative. >>> >> I did a small test. I compiled the SHA b55c064f of libvirt with a >> small modification. I enforced the lease time to be 2mins. Then I >> spawned a VM, and did the following: >> >> $ sudo journalctl -f | grep dnsmasq-dhcp >> Sep 24 19:31:13 lenovo dnsmasq-dhcp[9093]: DHCPDISCOVER(virbr1) >> 52:54:00:12:34:56 >> Sep 24 19:31:13 lenovo dnsmasq-dhcp[9093]: DHCPOFFER(virbr1) >> 192.168.123.76 52:54:00:12:34:56 >> Sep 24 19:31:13 lenovo dnsmasq-dhcp[9093]: DHCPREQUEST(virbr1) >> 192.168.123.76 52:54:00:12:34:56 >> Sep 24 19:31:13 lenovo dnsmasq-dhcp[9093]: DHCPACK(virbr1) >> 192.168.123.76 52:54:00:12:34:56 >> >> --- Now I pause the VM by sending 'stop' on the qemu monitor and wait. >> --- As soon as lease expires, I go back to the qemu monitor and >> un-pause the VM by sending 'cont' on the qemu monitor. >> >> Sep 24
Re: [libvirt] [PATCH v3 2/4] libxl: channels support
On 09/26/2016 10:44 PM, Jim Fehlig wrote:
> On 09/26/2016 11:33 AM, Joao Martins wrote:
>> And allow libxl to handle channel element which creates a Xen
>> console visible to the guest as a low-bandwitdh communication
>> channel. If type is PTY we also fetch the tty after boot using
>> libxl_channel_getinfo to fetch the tty path. On socket case,
>> we autogenerate a path if not specified in the XML. Path autogenerated
>> is slightly different from qemu driver: qemu stores also on
>> "channels/target" but it creates then a directory per domain with
>> each channel target name. libxl doesn't appear to have a clear
>> definition of private files associated with each domain, so for
>> simplicity we do it slightly different. On qemu each autogenerated
>> channel goes like:
>>
>> channels/target//
>>
>> Whereas for libxl:
>>
>> channels/target/-
>>
>> Should note that if path is not specified it won't persist,
>> existing only on live XML, unless user had initially specified it.
>> Since support for libxl channels only came on Xen >= 4.5 we therefore
>> need to conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
>>
>> After this patch and having a qemu guest agent:
>> $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
>>
>>
>>
>>
>>
>> $ virsh create domain.xml
>> $ echo '{"execute":"guest-network-get-interfaces"}' | socat
>> stdio,ignoreeof unix-connect:/tmp/channel
>>
>> {"execute":"guest-network-get-interfaces"}
>> {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
>> "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
>> "ip-address": "::1", "prefix": 128}], "hardware-address":
>> "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
>> [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
>> {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
>> "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
>> "sit0"}]}
>>
>> Signed-off-by: Joao Martins
>> ---
>> Since v2:
>> * Remove ret variable and do similar to other make functions.
>> * Have channelDir passed as an argument instead of driver.
>>
>> Since v1:
>> * Autogenerated paths, and updated commit message explaning it the different
>> naming. Despite per domain name being slightly different, parent
>> directory is same across both drivers.
>> ---
>> src/libxl/libxl_conf.c | 110
>> +++
>> src/libxl/libxl_conf.h | 3 ++
>> src/libxl/libxl_domain.c | 43 +-
>> src/libxl/libxl_driver.c | 7 +++
>> 4 files changed, 162 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 92faa44..4c94d54 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -88,6 +88,7 @@ libxlDriverConfigDispose(void *obj)
>> VIR_FREE(cfg->saveDir);
>> VIR_FREE(cfg->autoDumpDir);
>> VIR_FREE(cfg->lockManagerName);
>> +VIR_FREE(cfg->channelDir);
>> virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
>> }
>>
>> @@ -1348,6 +1349,8 @@ libxlDriverConfigNew(void)
>> goto error;
>> if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0)
>> goto error;
>> +if (VIR_STRDUP(cfg->channelDir, LIBXL_CHANNEL_DIR) < 0)
>> +goto error;
>>
>> if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) < 0)
>> goto error;
>> @@ -1499,6 +1502,107 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr
>> cfg,
>>
>> }
>>
>> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
>> +static int
>> +libxlPrepareChannel(virDomainChrDefPtr channel,
>> +const char *channelDir,
>> +const char *domainName)
>> +{
>> +if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
>> +channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>> +!channel->source.data.nix.path) {
>> +if (virAsprintf(&channel->source.data.nix.path,
>> +"%s/%s-%s", channelDir, domainName,
>> +channel->target.name ? channel->target.name
>> +: "unknown.sock") < 0)
>> +return -1;
>> +
>> +channel->source.data.nix.listen = true;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int
>> +libxlMakeChannel(virDomainChrDefPtr l_channel,
>> + libxl_device_channel *x_channel)
>> +{
>> +libxl_device_channel_init(x_channel);
>> +
>> +if (l_channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) {
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("channel target type not supported"));
>> +return -1;
>> +}
>> +
>> +switch (l_channel->source.type) {
>> +case VIR_DOMAIN_CHR_TYPE_PTY:
>> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
>> +break;
>> +case VIR_DOMAIN_CHR_TYPE_UNIX:
>> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
Re: [libvirt] [PATCH] Fix coding style issues.
On 09/26/2016 03:01 PM, Nitesh Konkar wrote:
> Signed-off-by: Nitesh Konkar
> ---
> src/uml/uml_conf.h | 2 +-
> src/vbox/vbox_common.h | 2 +-
> tests/qemumonitorjsontest.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
ACK and pushed w/ one slight adjustment below
Tks -
John
> diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h
> index 9a45d10..5da55e5 100644
> --- a/src/uml/uml_conf.h
> +++ b/src/uml/uml_conf.h
> @@ -35,7 +35,7 @@
> # include "vircommand.h"
> # include "virhash.h"
>
> -# define umlDebug(fmt, ...) do {} while(0)
> +# define umlDebug(fmt, ...) do {} while (0)
>
> # define UML_CPUMASK_LEN CPU_SETSIZE
>
> diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
> index c8787c3..b178878 100644
> --- a/src/vbox/vbox_common.h
> +++ b/src/vbox/vbox_common.h
> @@ -435,6 +435,6 @@ typedef nsISupports IKeyboard;
> } else {\
> result = -1;\
> } \
> -} while(0)
> +} while (0)
>
> #endif /* VBOX_COMMON_H */
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index cbc39c6..9823a9d 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -721,7 +721,7 @@ testQemuMonitorJSONAttachChardev(const void *data)
> goto cleanup; \
> if (qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test), \
> chrID, &chr) < 0) \
> -ret = fail ? ret : -1; \
> +ret = fail ? ret : -1; \
When you delete a space in the line you need to ensure the \ lines up at
the end of the line too...
> else\
> ret = fail ? -1 : ret; \
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] introduce pull backup
On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
> Add API to start/stop exporting disks for a backup and add qemu
> implementation.
>
> The latter is not complete yet. At least backup disks are not
> cleaned up and libvirt restart is not handled.
> ---
> examples/object-events/event-test.c | 3 +
> include/libvirt/libvirt-domain-backup.h | 45 +++
> include/libvirt/libvirt-domain.h| 3 +
> include/libvirt/libvirt.h | 1 +
> include/libvirt/virterror.h | 1 +
> po/POTFILES.in | 2 +
> src/Makefile.am | 3 +
> src/access/viraccessperm.c | 3 +-
> src/access/viraccessperm.h | 6 +
> src/conf/backup_conf.c | 295 ++
> src/conf/backup_conf.h | 85
> src/conf/domain_conf.c | 2 +-
> src/driver-hypervisor.h | 11 +
> src/libvirt-domain-backup.c | 86
> src/libvirt_private.syms| 6 +
> src/libvirt_public.syms | 2 +
> src/qemu/qemu_blockjob.c| 2 +
> src/qemu/qemu_conf.h| 1 +
> src/qemu/qemu_domain.h | 4 +
> src/qemu/qemu_driver.c | 684
>
> src/qemu/qemu_monitor.c | 32 ++
> src/qemu/qemu_monitor.h | 12 +
> src/qemu/qemu_monitor_json.c| 105 +
> src/qemu/qemu_monitor_json.h| 16 +
> src/remote/remote_driver.c | 2 +
> src/remote/remote_protocol.x| 33 +-
> src/util/virerror.c | 1 +
> tools/Makefile.am | 1 +
> tools/virsh-backup.c| 150 +++
> tools/virsh-backup.h| 28 ++
> tools/virsh-domain.c| 3 +-
> tools/virsh.c | 2 +
> tools/virsh.h | 1 +
> 33 files changed, 1627 insertions(+), 4 deletions(-)
> create mode 100644 include/libvirt/libvirt-domain-backup.h
> create mode 100644 src/conf/backup_conf.c
> create mode 100644 src/conf/backup_conf.h
> create mode 100644 src/libvirt-domain-backup.c
> create mode 100644 tools/virsh-backup.c
> create mode 100644 tools/virsh-backup.h
>
This patch fails make check/syntax-check :
GEN check-augeas-virtlogd
--- remote_protocol-structs 2016-09-26 08:01:11.645962427 -0400
+++ remote_protocol-struct-t3 2016-09-26 08:59:57.386094561 -0400
@@ -2080,6 +2080,21 @@
remote_nonnull_domain_snapshot snap;
u_int flags;
};
+struct remote_domain_backup_start_args {
+remote_nonnull_domain dom;
+remote_nonnull_string xml_desc;
+u_int flags;
+};
+struct remote_domain_backup_start_ret {
+intresult;
+};
+struct remote_domain_backup_stop_args {
+remote_nonnull_domain dom;
+u_int flags;
+};
+struct remote_domain_backup_stop_ret {
+intresult;
+};
struct remote_domain_open_console_args {
remote_nonnull_domain dom;
remote_string dev_name;
@@ -3169,4 +3184,6 @@
REMOTE_PROC_CONNECT_NODE_DEVICE_EVENT_DEREGISTER_ANY = 375,
REMOTE_PROC_NODE_DEVICE_EVENT_LIFECYCLE = 376,
REMOTE_PROC_NODE_DEVICE_EVENT_UPDATE = 377,
+REMOTE_PROC_DOMAIN_BACKUP_START = 378,
+REMOTE_PROC_DOMAIN_BACKUP_STOP = 379,
};
Makefile:11101: recipe for target 'remote_protocol-struct' failed
This also needs to be split up and perhaps regenerated as an RFC so that
debate can be had on your cover/.0 description.
Because it's also dependent upon an x-blockdev-del, it cannot be pushed
upstream to libvirt. I know qemu work continues w/r/t blockdev-add and
backups, but I don't follow it in detail (not enough time in the day!).
I'll scan the rest to provide a few comments... I really didn't dig
into algorithms too much.
> diff --git a/examples/object-events/event-test.c
> b/examples/object-events/event-test.c
> index 730cb8b..08490bb 100644
> --- a/examples/object-events/event-test.c
> +++ b/examples/object-events/event-test.c
> @@ -829,6 +829,9 @@ blockJobTypeToStr(int type)
>
> case VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT:
> return "active layer block commit";
> +
> +case VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP:
> +return "block backup";
> }
>
> return "unknown";
> diff --git a/include/libvirt/libvirt-domain-backup.h
> b/include/libvirt/libvirt-domain-backup.h
> new file mode 100644
> index 000..cd24995
> --- /dev/null
> +++ b/include/libvirt/libvirt-domain-backup.h
> @@ -0,0 +1,45 @@
> +/*
> + * libvirt-domain-backup.h
> + * Summary: APIs for management of domain backups
> + * Description: Provides APIs for the management of domain backups
> + * Author: Niko
Re: [libvirt] virsh can list ("virsh list" is empty) Guest domains created by "xl create DomU"
On 09/26/2016 04:44 AM, Volo M. wrote: > Hi, > > We use Centos 7 Xen hypervisor, start VMs with xl toolkit (libxl), but we > can't list with "virsh list" command DomUs created with "xl create ..." > command. > Can anybody, please, explain why this happens? Why does it work like this? For an explanation, start reading at paragraph 7 in this old blog post https://jfehlig.wordpress.com/2014/01/05/libvirt-support-for-xens-new-libxenlight-toolstack/ > It worked before with "libvirt+xm" toolkits, but xm toolkit was deprecated for > latest Xen sortware on Centos 7. > Can this be fixed ever? No, but it would be possible to add functionality similar to 'virsh qemu-attach' in the libvirt libxl driver. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] libxl: channels support
On 09/26/2016 11:33 AM, Joao Martins wrote:
> And allow libxl to handle channel element which creates a Xen
> console visible to the guest as a low-bandwitdh communication
> channel. If type is PTY we also fetch the tty after boot using
> libxl_channel_getinfo to fetch the tty path. On socket case,
> we autogenerate a path if not specified in the XML. Path autogenerated
> is slightly different from qemu driver: qemu stores also on
> "channels/target" but it creates then a directory per domain with
> each channel target name. libxl doesn't appear to have a clear
> definition of private files associated with each domain, so for
> simplicity we do it slightly different. On qemu each autogenerated
> channel goes like:
>
> channels/target//
>
> Whereas for libxl:
>
> channels/target/-
>
> Should note that if path is not specified it won't persist,
> existing only on live XML, unless user had initially specified it.
> Since support for libxl channels only came on Xen >= 4.5 we therefore
> need to conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
>
> After this patch and having a qemu guest agent:
> $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
>
>
>
>
>
> $ virsh create domain.xml
> $ echo '{"execute":"guest-network-get-interfaces"}' | socat
> stdio,ignoreeof unix-connect:/tmp/channel
>
> {"execute":"guest-network-get-interfaces"}
> {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
> "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
> "ip-address": "::1", "prefix": 128}], "hardware-address":
> "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
> [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
> {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
> "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
> "sit0"}]}
>
> Signed-off-by: Joao Martins
> ---
> Since v2:
> * Remove ret variable and do similar to other make functions.
> * Have channelDir passed as an argument instead of driver.
>
> Since v1:
> * Autogenerated paths, and updated commit message explaning it the different
> naming. Despite per domain name being slightly different, parent
> directory is same across both drivers.
> ---
> src/libxl/libxl_conf.c | 110
> +++
> src/libxl/libxl_conf.h | 3 ++
> src/libxl/libxl_domain.c | 43 +-
> src/libxl/libxl_driver.c | 7 +++
> 4 files changed, 162 insertions(+), 1 deletion(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 92faa44..4c94d54 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -88,6 +88,7 @@ libxlDriverConfigDispose(void *obj)
> VIR_FREE(cfg->saveDir);
> VIR_FREE(cfg->autoDumpDir);
> VIR_FREE(cfg->lockManagerName);
> +VIR_FREE(cfg->channelDir);
> virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
> }
>
> @@ -1348,6 +1349,8 @@ libxlDriverConfigNew(void)
> goto error;
> if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0)
> goto error;
> +if (VIR_STRDUP(cfg->channelDir, LIBXL_CHANNEL_DIR) < 0)
> +goto error;
>
> if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) < 0)
> goto error;
> @@ -1499,6 +1502,107 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr
> cfg,
>
> }
>
> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
> +static int
> +libxlPrepareChannel(virDomainChrDefPtr channel,
> +const char *channelDir,
> +const char *domainName)
> +{
> +if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
> +channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
> +!channel->source.data.nix.path) {
> +if (virAsprintf(&channel->source.data.nix.path,
> +"%s/%s-%s", channelDir, domainName,
> +channel->target.name ? channel->target.name
> +: "unknown.sock") < 0)
> +return -1;
> +
> +channel->source.data.nix.listen = true;
> +}
> +
> +return 0;
> +}
> +
> +static int
> +libxlMakeChannel(virDomainChrDefPtr l_channel,
> + libxl_device_channel *x_channel)
> +{
> +libxl_device_channel_init(x_channel);
> +
> +if (l_channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("channel target type not supported"));
> +return -1;
> +}
> +
> +switch (l_channel->source.type) {
> +case VIR_DOMAIN_CHR_TYPE_PTY:
> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
> +break;
> +case VIR_DOMAIN_CHR_TYPE_UNIX:
> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
> +if (VIR_STRDUP(x_channel->u.socket.path,
> + l_channel->source.data.nix.path) < 0)
> +return -1;
> +break;
> +defa
Re: [libvirt] [PATCH 1/3] qemu: store guest visible disk size from qemu monitor block info
On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
> ---
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_monitor_json.c | 17 +
> tests/qemumonitorjsontest.c | 36
> 3 files changed, 54 insertions(+)
>
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 13c0372..ea57111 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -337,6 +337,7 @@ struct qemuDomainDiskInfo {
> bool tray_open;
> bool empty;
> int io_status;
> +unsigned long long guest_size;
a/k/a: 'virtual-size' in qemu and 'capacity' in libvirt
I probably would stick with capacity or something that says "disk"
rather than "guest".
> };
>
Trying to determine/decide whether this patch should be "separated" and
perhaps made usable with/for the existing callers or whether patch 3
should use the block stats (qemuDomainGetStatsBlock) which already
handles some details that could be missing here (at least w/r/t backing
chains).
Another option is adjusting the qemuMonitorJSONDiskNameLookup code to be
more multi-purpose since it's using the "query-block" for
*BlockPullCommon (from Rebase and Pull) as well as from *BlockCommit and
it already checks/expects both "inserted" and "image" to be present in
order to return that name.
> typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate;
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 3d0a120..7903b47 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1800,6 +1800,8 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>
> for (i = 0; i < virJSONValueArraySize(devices); i++) {
> virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
> +virJSONValuePtr inserted;
> +virJSONValuePtr image;
> struct qemuDomainDiskInfo *info;
> const char *thisdev;
> const char *status;
> @@ -1854,6 +1856,21 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
> if (info->io_status < 0)
> goto cleanup;
> }
> +
> +if ((inserted = virJSONValueObjectGetObject(dev, "inserted"))) {
> +if (!(image = virJSONValueObjectGetObject(inserted, "image"))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("cannot read 'inserted/image' value"));
> +goto cleanup;
> +}
Other code that checks "inserted" and "image" doesn't fail - it just
ignores. If there's only going to be one consumer, then I don't believe
we want a failure such as this to affect other callers that may not
care. That could mean having some sort of "marker" in the returned
buffer that the fetch of "virtual-size" did occur (or just using not
zero). It would be a shame to have some unexpected failures for a field
that nothing else uses especially since the *GetBlockInfo is being used
during process launch and reconnect (via qemuProcessRefreshDisks).
Futhermore, what if there's a "backing-image" for "this" particular
path? Will that be important for the pull backups support? Without
looking at patch 3 I would think so...
John
> +
> +if (virJSONValueObjectGetNumberUlong(image, "virtual-size",
> +&info->guest_size) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("cannot read 'inserted/image/virtual-size'
> value"));
> +goto cleanup;
> +}
> +}
> }
>
> ret = 0;
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index d3ec3b1..d1922fd 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -50,6 +50,23 @@ const char *queryBlockReply =
> "\"locked\": false,"
> "\"removable\": false,"
> "\"inserted\": {"
> +"\"image\": {"
> +"\"virtual-size\": 68719476736,"
> +"\"filename\": \"/home/zippy/work/tmp/gentoo.qcow2\","
> +"\"cluster-size\": 65536,"
> +"\"format\": \"qcow2\","
> +"\"actual-size\": 156901376,"
> +"\"format-specific\": {"
> +"\"type\": \"qcow2\","
> +"\"data\": {"
> +"\"compat\": \"1.1\","
> +"\"lazy-refcounts\": true,"
> +"\"refcount-bits\": 16,"
> +"\"corrupt\": false"
> +"}"
> +"},"
> +"\"dirty-flag\": false"
> +"},"
> "\"iops_rd\": 5,"
> "\"iops_wr\": 6,"
> "\"ro\": false,"
> @@ -78,6 +95,13 @@ const char *queryBlockReply =
> "\"locked\": false,"
> "\"removable\": false,"
> "\"inserted\
Re: [libvirt] [PATCH 2/3] qemu: special error code in case of no job on cancel block job
On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
> Special error code helps gracefully handle race conditions on
> blockjob cancelling. Consider for example pull backup. We want
> to stop it and in the process we cancel blockjob for some of the
> exported disks. But at the time this command reaches qemu blockjob
> fail for some reason and cancel result and result of stopping
> will be an error with quite unexpecte message - no such job (sort of).
> Instead we can detect this race and treat as correct cancelling
> and wait until fail event will arrive. Then we can report corect
> error message with some details from qemu probably.
>
> Users of domainBlockJobAbort can be affected as -2 is new possible
> return code. Not sure if I should stick to the original behaviour in
> this case.
> ---
> src/qemu/qemu_monitor.c | 5 +
> src/qemu/qemu_monitor_json.c | 11 +++
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
I haven't looked forward to patch 3/3 yet, but perhaps it'd be better in
the "caller" that cares to :
if (qemu*() < 0) {
virErrorPtr err = virGetLastError();
if (err && err->code == VIR_ERR_OPERATION_INVALID) {
/* Do something specific */
}
}
Returning -2 alters virDomainBlockJobAbort expected return values and
well that's probably not a good idea since we don't know if some caller
has said (if virDomainBlockJobAbort() == -1) instead of <= -1
John
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 4171914..944856d 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3335,6 +3335,11 @@ qemuMonitorBlockStream(qemuMonitorPtr mon,
> }
>
>
> +/* return:
> + * 0 in case of success
> + * -1 in case of general error
> + * -2 in case there is no such job
> + */
> int
> qemuMonitorBlockJobCancel(qemuMonitorPtr mon,
>const char *device,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 7903b47..42b05c4 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4351,6 +4351,7 @@ qemuMonitorJSONBlockJobError(virJSONValuePtr reply,
> if (qemuMonitorJSONErrorIsClass(error, "DeviceNotActive")) {
> virReportError(VIR_ERR_OPERATION_INVALID,
> _("No active operation on device: %s"), device);
> +return -2;
> } else if (qemuMonitorJSONErrorIsClass(error, "DeviceInUse")) {
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("Device %s in use"), device);
> @@ -4408,6 +4409,11 @@ qemuMonitorJSONBlockStream(qemuMonitorPtr mon,
> }
>
>
> +/* return:
> + * 0 in case of success
> + * -1 in case of general error
> + * -2 in case there is no such job
> + */
> int
> qemuMonitorJSONBlockJobCancel(qemuMonitorPtr mon,
>const char *device,
> @@ -4426,10 +4432,7 @@ qemuMonitorJSONBlockJobCancel(qemuMonitorPtr mon,
> if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> goto cleanup;
>
> -if (qemuMonitorJSONBlockJobError(reply, cmd_name, device) < 0)
> -goto cleanup;
> -
> -ret = 0;
> +ret = qemuMonitorJSONBlockJobError(reply, cmd_name, device);
>
> cleanup:
> virJSONValueFree(cmd);
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix coding style issues.
Signed-off-by: Nitesh Konkar
---
src/uml/uml_conf.h | 2 +-
src/vbox/vbox_common.h | 2 +-
tests/qemumonitorjsontest.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h
index 9a45d10..5da55e5 100644
--- a/src/uml/uml_conf.h
+++ b/src/uml/uml_conf.h
@@ -35,7 +35,7 @@
# include "vircommand.h"
# include "virhash.h"
-# define umlDebug(fmt, ...) do {} while(0)
+# define umlDebug(fmt, ...) do {} while (0)
# define UML_CPUMASK_LEN CPU_SETSIZE
diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
index c8787c3..b178878 100644
--- a/src/vbox/vbox_common.h
+++ b/src/vbox/vbox_common.h
@@ -435,6 +435,6 @@ typedef nsISupports IKeyboard;
} else {\
result = -1;\
} \
-} while(0)
+} while (0)
#endif /* VBOX_COMMON_H */
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index cbc39c6..9823a9d 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -721,7 +721,7 @@ testQemuMonitorJSONAttachChardev(const void *data)
goto cleanup; \
if (qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test), \
chrID, &chr) < 0) \
-ret = fail ? ret : -1; \
+ret = fail ? ret : -1; \
else\
ret = fail ? -1 : ret; \
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virsh can list ("virsh list" is empty) Guest domains created by "xl create DomU"
Hi, We use Centos 7 Xen hypervisor, start VMs with xl toolkit (libxl), but we can't list with "virsh list" command DomUs created with "xl create ..." command. Can anybody, please, explain why this happens? Why does it work like this? It worked before with "libvirt+xm" toolkits, but xm toolkit was deprecated for latest Xen sortware on Centos 7. Can this be fixed ever? Thank you. Example: [root@c7xen2 ~]# xl create /onapp/config/w62tp7db1txk12 ... [root@c7xen2 ~]# xl list NameID Mem VCPUs State Time(s) Domain-0 0 400 2 r- 102.0 w62tp7db1txk12 1 512 1 -b 2.6 [root@c7xen2 ~]# virsh list IdName State 0 Domain-0 running [root@c7xen2 ~]# -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/4] xlconfigtest: add test for channel conversion
Signed-off-by: Joao Martins
Acked-by: Jim Fehlig
---
tests/xlconfigdata/test-channel-pty.cfg | 13
tests/xlconfigdata/test-channel-pty.xml | 33 +++
tests/xlconfigdata/test-channel-unix.cfg | 13
tests/xlconfigdata/test-channel-unix.xml | 34
tests/xlconfigtest.c | 4
5 files changed, 97 insertions(+)
create mode 100644 tests/xlconfigdata/test-channel-pty.cfg
create mode 100644 tests/xlconfigdata/test-channel-pty.xml
create mode 100644 tests/xlconfigdata/test-channel-unix.cfg
create mode 100644 tests/xlconfigdata/test-channel-unix.xml
diff --git a/tests/xlconfigdata/test-channel-pty.cfg
b/tests/xlconfigdata/test-channel-pty.cfg
new file mode 100644
index 000..b20e487
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-pty.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest1"
+uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283"
+maxmem = 512
+memory = 512
+vcpus = 1
+localtime = 0
+on_poweroff = "preserve"
+on_reboot = "restart"
+on_crash = "preserve"
+vif = [ "mac=5a:36:0e:be:00:09" ]
+bootloader = "/usr/bin/pygrub"
+disk = [
"format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2"
]
+channel = [ "connection=pty,name=org.qemu.guest_agent.0" ]
diff --git a/tests/xlconfigdata/test-channel-pty.xml
b/tests/xlconfigdata/test-channel-pty.xml
new file mode 100644
index 000..17d0c67
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-pty.xml
@@ -0,0 +1,33 @@
+
+ XenGuest1
+ 45b60f51-88a9-47a8-a3b3-5e66d71b2283
+ 524288
+ 524288
+ 1
+ /usr/bin/pygrub
+
+linux
+
+
+ preserve
+ restart
+ preserve
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/xlconfigdata/test-channel-unix.cfg
b/tests/xlconfigdata/test-channel-unix.cfg
new file mode 100644
index 000..ada7001
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-unix.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest1"
+uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283"
+maxmem = 512
+memory = 512
+vcpus = 1
+localtime = 0
+on_poweroff = "preserve"
+on_reboot = "restart"
+on_crash = "preserve"
+vif = [ "mac=5a:36:0e:be:00:09" ]
+bootloader = "/usr/bin/pygrub"
+disk = [
"format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2"
]
+channel = [
"connection=socket,path=/path/to/socket,name=org.qemu.guest_agent.0" ]
diff --git a/tests/xlconfigdata/test-channel-unix.xml
b/tests/xlconfigdata/test-channel-unix.xml
new file mode 100644
index 000..8f4eaa2
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-unix.xml
@@ -0,0 +1,34 @@
+
+ XenGuest1
+ 45b60f51-88a9-47a8-a3b3-5e66d71b2283
+ 524288
+ 524288
+ 1
+ /usr/bin/pygrub
+
+linux
+
+
+ preserve
+ restart
+ preserve
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index d99f887..31892da 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -268,6 +268,10 @@ mymain(void)
DO_TEST_FORMAT("paravirt-cmdline-bogus-extra-root", false);
DO_TEST("rbd-multihost-noauth");
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+DO_TEST("channel-pty");
+DO_TEST("channel-unix");
+#endif
#ifdef LIBXL_HAVE_BUILDINFO_SERIAL_LIST
DO_TEST("fullvirt-multiserial");
#endif
--
2.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/4] conf: add xen type for channels
So far only guestfwd and virtio were supported. Add an additional
for Xen as libxl channels create Xen console visible to the guest.
Signed-off-by: Joao Martins
---
Changes since v2:
* Add relevant documentation about target type xen.
---
docs/formatdomain.html.in | 10 ++
docs/schemas/domaincommon.rng | 11 +++
src/conf/domain_conf.c| 18 ++
src/conf/domain_conf.h| 1 +
src/qemu/qemu_command.c | 1 +
5 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f48a4d8..129ba62 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5980,6 +5980,16 @@ qemu-kvm -net nic,model=? /dev/null
Possible values for the state attribute are
connected and disconnected.
+ xen
+ Paravirtualized xen channel. Channel is exposed in the guest as a
+xen console but identified with a name. The setup of the channel
+depends to guest own rules and can live in a arbitrary path (for more
+info, please see http://xenbits.xen.org/docs/unstable/misc/channel.txt";>http://xenbits.xen.org/docs/unstable/misc/channel.txt).
+Channel source path semantics are the same as the virtio target type.
+Although state attribute is not provided as xen channels
+lack the necessary probing mechanism.
+Since 2.3.0
+
spicevmc
Paravirtualized SPICE channel. The domain must also have a
SPICE server as a graphics
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 95c7882..6eeb4e9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3690,6 +3690,16 @@
+
+
+
+xen
+
+
+
+
+
+
@@ -3698,6 +3708,7 @@
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a06da46..7972a4e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -434,7 +434,8 @@ VIR_ENUM_IMPL(virDomainChrChannelTarget,
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST,
"none",
"guestfwd",
- "virtio")
+ "virtio",
+ "xen")
VIR_ENUM_IMPL(virDomainChrConsoleTarget,
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST,
@@ -2068,6 +2069,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
VIR_FREE(def->target.addr);
break;
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
VIR_FREE(def->target.name);
break;
@@ -9923,10 +9925,12 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def,
virSocketAddrSetPort(def->target.addr, port);
break;
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
def->target.name = virXMLPropString(cur, "name");
-if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
+if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
(stateStr = virXMLPropString(cur, "state"))) {
int tmp;
@@ -10217,7 +10221,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr
def,
/* path can be auto generated */
if (!path &&
(!chr_def ||
- chr_def->targetType !=
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)) {
+ (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
+ chr_def->targetType !=
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing source path attribute for char device"));
goto error;
@@ -14417,6 +14422,7 @@ virDomainChrEquals(virDomainChrDefPtr src,
if (src->targetType != tgt->targetType)
return false;
switch ((virDomainChrChannelTargetType) src->targetType) {
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
return STREQ_NULLABLE(src->target.name, tgt->target.name);
break;
@@ -18434,6 +18440,8 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr
src,
}
switch (src->targetType) {
+
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
if (STRNEQ_NULLABLE(src->target.name, dst->target.name)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -21556,11 +21564,13 @@ virDomainChrDefFormat(virBufferPtr buf,
break;
}
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
if (def->target.name)
[libvirt] [PATCH v3 0/4] libxl: channels support
Hey,
This v3 from channel series with latest comments addressed. Difference to
qemu driver would be only the autogenerated path being slightly different.
Channels have been on xen toolstack since Xen 4.5 and this small series
adds support for it, including xenconfig conversion and appropriate tests.
After this series it's possible to do this:
(assuming correct configuration of qemu agent in the guest)
$ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
$ virsh create domain.xml
$ echo '{"execute":"guest-network-get-interfaces"}' | socat
stdio,ignoreeof unix-connect:/tmp/channel
{"execute":"guest-network-get-interfaces"}
{"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
"ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
"ip-address": "::1", "prefix": 128}], "hardware-address":
"00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
[{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
{"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
"prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
"sit0"}]}
Thanks,
Joao
Joao Martins (4):
conf: add xen type for channels
libxl: channels support
xenconfig: channels conversion support
xlconfigtest: add test for channel conversion
docs/formatdomain.html.in| 10 ++
docs/schemas/domaincommon.rng| 11 ++
src/conf/domain_conf.c | 18 +++-
src/conf/domain_conf.h | 1 +
src/libxl/libxl_conf.c | 110 +++
src/libxl/libxl_conf.h | 3 +
src/libxl/libxl_domain.c | 43 +++-
src/libxl/libxl_driver.c | 7 ++
src/qemu/qemu_command.c | 1 +
src/xenconfig/xen_xl.c | 176 +++
tests/xlconfigdata/test-channel-pty.cfg | 13 +++
tests/xlconfigdata/test-channel-pty.xml | 33 ++
tests/xlconfigdata/test-channel-unix.cfg | 13 +++
tests/xlconfigdata/test-channel-unix.xml | 34 ++
tests/xlconfigtest.c | 4 +
15 files changed, 472 insertions(+), 5 deletions(-)
create mode 100644 tests/xlconfigdata/test-channel-pty.cfg
create mode 100644 tests/xlconfigdata/test-channel-pty.xml
create mode 100644 tests/xlconfigdata/test-channel-unix.cfg
create mode 100644 tests/xlconfigdata/test-channel-unix.xml
--
2.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/4] xenconfig: channels conversion support
Add support for formating/parsing libxl channels.
Syntax on xen libxl goes as following:
channel=["connection=pty|socket,path=/path/to/socket,name=XXX",...]
Signed-off-by: Joao Martins
Acked-by: Jim Fehlig
---
Changes since v1:
* Move path to UNIX case.
---
src/xenconfig/xen_xl.c | 176 +
1 file changed, 176 insertions(+)
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 4615bfd..a06983e 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -685,6 +685,93 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int
+xenParseXLChannel(virConfPtr conf, virDomainDefPtr def)
+{
+virConfValuePtr list = virConfGetValue(conf, "channel");
+virDomainChrDefPtr channel = NULL;
+char *name = NULL;
+char *path = NULL;
+
+if (list && list->type == VIR_CONF_LIST) {
+list = list->list;
+while (list) {
+char type[10];
+char *key;
+
+if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+goto skipchannel;
+
+key = list->str;
+while (key) {
+char *data;
+char *nextkey = strchr(key, ',');
+
+if (!(data = strchr(key, '=')))
+goto skipchannel;
+data++;
+
+if (STRPREFIX(key, "connection=")) {
+int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("connection %s too big"), data);
+goto skipchannel;
+}
+} else if (STRPREFIX(key, "name=")) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(name);
+if (VIR_STRNDUP(name, data, len) < 0)
+goto cleanup;
+} else if (STRPREFIX(key, "path=")) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(path);
+if (VIR_STRNDUP(path, data, len) < 0)
+goto cleanup;
+}
+
+while (nextkey && (nextkey[0] == ',' ||
+ nextkey[0] == ' ' ||
+ nextkey[0] == '\t'))
+nextkey++;
+key = nextkey;
+}
+
+if (!(channel = virDomainChrDefNew()))
+goto cleanup;
+
+if (STRPREFIX(type, "socket")) {
+channel->source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+channel->source.data.nix.path = path;
+channel->source.data.nix.listen = 1;
+} else if (STRPREFIX(type, "pty")) {
+channel->source.type = VIR_DOMAIN_CHR_TYPE_PTY;
+VIR_FREE(path);
+} else {
+goto cleanup;
+}
+
+channel->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL;
+channel->targetType = VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN;
+channel->target.name = name;
+
+if (VIR_APPEND_ELEMENT(def->channels, def->nchannels, channel) < 0)
+goto cleanup;
+
+skipchannel:
+list = list->next;
+}
+}
+
+return 0;
+
+ cleanup:
+virDomainChrDefFree(channel);
+VIR_FREE(path);
+VIR_FREE(name);
+return -1;
+}
+
virDomainDefPtr
xenParseXL(virConfPtr conf,
virCapsPtr caps,
@@ -720,6 +807,9 @@ xenParseXL(virConfPtr conf,
if (xenParseXLUSBController(conf, def) < 0)
goto cleanup;
+if (xenParseXLChannel(conf, def) < 0)
+goto cleanup;
+
if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
xmlopt, NULL) < 0)
goto cleanup;
@@ -1347,6 +1437,89 @@ xenFormatXLUSB(virConfPtr conf,
return -1;
}
+static int
+xenFormatXLChannel(virConfValuePtr list, virDomainChrDefPtr channel)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int sourceType = channel->source.type;
+virConfValuePtr val, tmp;
+
+/* connection */
+virBufferAddLit(&buf, "connection=");
+switch (sourceType) {
+case VIR_DOMAIN_CHR_TYPE_PTY:
+virBufferAddLit(&buf, "pty,");
+break;
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+virBufferAddLit(&buf, "socket,");
+/* path */
+if (channel->source.data.nix.path)
+virBufferAsprintf(&buf, "path=%s,",
+ channel->source.data.nix.path);
+break;
+default:
+goto cleanup;
+}
+
+/* name */
+virBufferAsprintf(&buf, "name=%s", channel->target.name);
+
+if (VIR_ALLOC(val) < 0)
+goto cleanup;
+
+val->type
[libvirt] [PATCH v3 2/4] libxl: channels support
And allow libxl to handle channel element which creates a Xen
console visible to the guest as a low-bandwitdh communication
channel. If type is PTY we also fetch the tty after boot using
libxl_channel_getinfo to fetch the tty path. On socket case,
we autogenerate a path if not specified in the XML. Path autogenerated
is slightly different from qemu driver: qemu stores also on
"channels/target" but it creates then a directory per domain with
each channel target name. libxl doesn't appear to have a clear
definition of private files associated with each domain, so for
simplicity we do it slightly different. On qemu each autogenerated
channel goes like:
channels/target//
Whereas for libxl:
channels/target/-
Should note that if path is not specified it won't persist,
existing only on live XML, unless user had initially specified it.
Since support for libxl channels only came on Xen >= 4.5 we therefore
need to conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
After this patch and having a qemu guest agent:
$ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
$ virsh create domain.xml
$ echo '{"execute":"guest-network-get-interfaces"}' | socat
stdio,ignoreeof unix-connect:/tmp/channel
{"execute":"guest-network-get-interfaces"}
{"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
"ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
"ip-address": "::1", "prefix": 128}], "hardware-address":
"00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
[{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
{"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
"prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
"sit0"}]}
Signed-off-by: Joao Martins
---
Since v2:
* Remove ret variable and do similar to other make functions.
* Have channelDir passed as an argument instead of driver.
Since v1:
* Autogenerated paths, and updated commit message explaning it the different
naming. Despite per domain name being slightly different, parent
directory is same across both drivers.
---
src/libxl/libxl_conf.c | 110 +++
src/libxl/libxl_conf.h | 3 ++
src/libxl/libxl_domain.c | 43 +-
src/libxl/libxl_driver.c | 7 +++
4 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 92faa44..4c94d54 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -88,6 +88,7 @@ libxlDriverConfigDispose(void *obj)
VIR_FREE(cfg->saveDir);
VIR_FREE(cfg->autoDumpDir);
VIR_FREE(cfg->lockManagerName);
+VIR_FREE(cfg->channelDir);
virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
}
@@ -1348,6 +1349,8 @@ libxlDriverConfigNew(void)
goto error;
if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0)
goto error;
+if (VIR_STRDUP(cfg->channelDir, LIBXL_CHANNEL_DIR) < 0)
+goto error;
if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) < 0)
goto error;
@@ -1499,6 +1502,107 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
}
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+static int
+libxlPrepareChannel(virDomainChrDefPtr channel,
+const char *channelDir,
+const char *domainName)
+{
+if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
+channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
+!channel->source.data.nix.path) {
+if (virAsprintf(&channel->source.data.nix.path,
+"%s/%s-%s", channelDir, domainName,
+channel->target.name ? channel->target.name
+: "unknown.sock") < 0)
+return -1;
+
+channel->source.data.nix.listen = true;
+}
+
+return 0;
+}
+
+static int
+libxlMakeChannel(virDomainChrDefPtr l_channel,
+ libxl_device_channel *x_channel)
+{
+libxl_device_channel_init(x_channel);
+
+if (l_channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("channel target type not supported"));
+return -1;
+}
+
+switch (l_channel->source.type) {
+case VIR_DOMAIN_CHR_TYPE_PTY:
+x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
+break;
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
+if (VIR_STRDUP(x_channel->u.socket.path,
+ l_channel->source.data.nix.path) < 0)
+return -1;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("channel source type not supported"));
+break;
+}
+
+if (!l_channel->target.name) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("channel target name missing
[libvirt] [PATCH v3] qemu-migration: Disallow migration of read only disk
Currently Libvirt allows attempts to migrate read only disks. Qemu cannot
handle this as read only
disks cannot be written to on the destination system. The end result is a
cryptic error message
and a failed migration.
This patch causes migration to fail earlier and provides a meaningful error
message stating that
migrating read only disks is not supported.
Signed-off-by: Corey S. McQuay
Reviewed-by: Jason J. Herne
Reviewed-by: Boris Fiuczynski
---
src/qemu/qemu_migration.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e451ef6..c8fb7ec 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1764,6 +1764,12 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
/* check whether disk should be migrated */
if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks))
continue;
+
+if (disk->src->readonly) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+_("Cannot migrate read-only disk %s"), disk->dst);
+goto cleanup;
+}
VIR_FREE(diskAlias);
if (!(diskAlias = qemuAliasFromDisk(disk)))
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] libxl: channels support
On 09/26/2016 05:27 PM, Jim Fehlig wrote:
> On 09/26/2016 09:30 AM, Joao Martins wrote:
>> On 09/26/2016 03:21 PM, Jim Fehlig wrote:
>>> On 09/25/2016 01:13 PM, Joao Martins wrote:
On 09/25/2016 07:55 PM, Joao Martins wrote:
> On 09/24/2016 12:15 AM, Joao Martins wrote:
>> On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig
>> wrote:
>>> On 09/22/2016 01:53 PM, Joao Martins wrote:
[snip]
int
-libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
+libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainDefPtr def,
libxl_ctx *ctx,
libxl_domain_config *d_config)
{
+virPortAllocatorPtr graphicsports =
>>> driver->reservedGraphicsPorts;
+
>>> Spurious change?
>> This (and the other two below) was intended, as I needed
>> channelDir. and instead of having yet another argument, I
>> passed driver instead as graphics port was using it too.
>>
>> But I could use the macro directly, or add another argument if you
>> prefer.
> Hmm, I can just avoid passing driver and have cfg->channelDir added as an
> argument. I just noticed that I am unnecessarily doing
> libxlDriverConfigGet
> twice and perhaps if a third argument is added in the future then probably
> consider having driver be passed as an argument?
Or even better have cfg as the function argument instead to allow also
removing
"ctx" argument. Both channelDir and ctx are stored in cfg. This way we
reduce
the number of arguments plus allow future usage on other functions called
from
libxlBuildDomainConfig.
>>> Yep, I think that is fine. We primarily want to avoid making
>>> libxlBuildDomainConfig difficult to call from the unit tests. I realize we
>>> don't
>>> currently do that, but the eventual plan is to test the conversion of
>>> virDomainDef to libxl_domain_config. danpb did some initial work on that
>>> quite
>>> some time ago, see commit 5da28f24.
>> Ah nice to know, I wasn't aware of that work. This cover letter
>> (https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html)
>> explains it
>> all too.
>
> Some of those patches were pushed. I did some followup work
>
> https://www.redhat.com/archives/libvir-list/2014-September/msg00180.html
>
> which also resulted in some patches being pushed. But the meaty parts of the
> series that actually provide the conversion tests were never committed. The
> last
> attempt to revive the work also stalled
>
> https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html
>
> Thinking about it again, I seems the best way forward is something along the
> lines of option 3 described in that thread
>
> "make use of new functionality in Xen 4.5 such as
> libxl_domain_config_from_json() and libxl_domain_config_compare()"
Interesting, the latter though (libxl_domain_config_compare) doesn't appear to
be implemented on 4.7 (or upcoming 4.8) and sounds like even if implemented that
it would rule out most of the versions :( Probably worked out with a shim for
older versions that implement equivalent of libxl_domain_config_compare().
>> What I am in this patch is clearly opposite the effort of commit
>> 5da28f24 and a6abdbf. But now I am not sure if what I proposed in my earlier
>> paragrah is any different: we can probably get away with a mock of
>> libxlDriverConfig but not sure.
>
> I suppose mocking it would be easier if libxl_ctx_alloc supported a dummy
> mode.
> I.e. if this bug was fixed
>
> https://bugs.xenproject.org/xen/bug/41
>
>> In the end sounds like just adding channelDir to
>> the function arguments might end up being better?
>
> IMO that is probably the best approach until we have the conversion tests
> figured out.
Cool, thanks for the feedback! Let me submit v3 with these fixed.
>
> BTW, can channels be hot (un)plugged? If so, it's another argument for just
> passing the channelDir. Future external callers of libxlMakeChannel() would
> have
> access to the libxlDriverConfig object, and hence channelDir.
AFAICT there's no API for hotplugging channels, very much like serials/consoles
can't be too hotplugged.
Joao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] libxl: channels support
On 09/26/2016 09:30 AM, Joao Martins wrote:
>
> On 09/26/2016 03:21 PM, Jim Fehlig wrote:
>> On 09/25/2016 01:13 PM, Joao Martins wrote:
>>> On 09/25/2016 07:55 PM, Joao Martins wrote:
On 09/24/2016 12:15 AM, Joao Martins wrote:
> On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig
> wrote:
>> On 09/22/2016 01:53 PM, Joao Martins wrote:
>>> [snip]
>>> int
>>> -libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>>> +libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
>>> virDomainDefPtr def,
>>> libxl_ctx *ctx,
>>> libxl_domain_config *d_config)
>>> {
>>> +virPortAllocatorPtr graphicsports =
>> driver->reservedGraphicsPorts;
>>> +
>> Spurious change?
> This (and the other two below) was intended, as I needed
> channelDir. and instead of having yet another argument, I
> passed driver instead as graphics port was using it too.
>
> But I could use the macro directly, or add another argument if you prefer.
Hmm, I can just avoid passing driver and have cfg->channelDir added as an
argument. I just noticed that I am unnecessarily doing libxlDriverConfigGet
twice and perhaps if a third argument is added in the future then probably
consider having driver be passed as an argument?
>>> Or even better have cfg as the function argument instead to allow also
>>> removing
>>> "ctx" argument. Both channelDir and ctx are stored in cfg. This way we
>>> reduce
>>> the number of arguments plus allow future usage on other functions called
>>> from
>>> libxlBuildDomainConfig.
>> Yep, I think that is fine. We primarily want to avoid making
>> libxlBuildDomainConfig difficult to call from the unit tests. I realize we
>> don't
>> currently do that, but the eventual plan is to test the conversion of
>> virDomainDef to libxl_domain_config. danpb did some initial work on that
>> quite
>> some time ago, see commit 5da28f24.
> Ah nice to know, I wasn't aware of that work. This cover letter
> (https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html) explains
> it
> all too.
Some of those patches were pushed. I did some followup work
https://www.redhat.com/archives/libvir-list/2014-September/msg00180.html
which also resulted in some patches being pushed. But the meaty parts of the
series that actually provide the conversion tests were never committed. The last
attempt to revive the work also stalled
https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html
Thinking about it again, I seems the best way forward is something along the
lines of option 3 described in that thread
"make use of new functionality in Xen 4.5 such as
libxl_domain_config_from_json() and libxl_domain_config_compare()"
> What I am in this patch is clearly opposite the effort of commit
> 5da28f24 and a6abdbf. But now I am not sure if what I proposed in my earlier
> paragrah is any different: we can probably get away with a mock of
> libxlDriverConfig but not sure.
I suppose mocking it would be easier if libxl_ctx_alloc supported a dummy mode.
I.e. if this bug was fixed
https://bugs.xenproject.org/xen/bug/41
> In the end sounds like just adding channelDir to
> the function arguments might end up being better?
IMO that is probably the best approach until we have the conversion tests
figured out.
BTW, can channels be hot (un)plugged? If so, it's another argument for just
passing the channelDir. Future external callers of libxlMakeChannel() would have
access to the libxlDriverConfig object, and hence channelDir.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] libxl: channels support
On 09/26/2016 03:21 PM, Jim Fehlig wrote:
> On 09/25/2016 01:13 PM, Joao Martins wrote:
>> On 09/25/2016 07:55 PM, Joao Martins wrote:
>>> On 09/24/2016 12:15 AM, Joao Martins wrote:
On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig
wrote:
> On 09/22/2016 01:53 PM, Joao Martins wrote:
>> [snip]
>> int
>> -libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>> +libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
>> virDomainDefPtr def,
>> libxl_ctx *ctx,
>> libxl_domain_config *d_config)
>> {
>> +virPortAllocatorPtr graphicsports =
> driver->reservedGraphicsPorts;
>> +
> Spurious change?
This (and the other two below) was intended, as I needed
channelDir. and instead of having yet another argument, I
passed driver instead as graphics port was using it too.
But I could use the macro directly, or add another argument if you prefer.
>>> Hmm, I can just avoid passing driver and have cfg->channelDir added as an
>>> argument. I just noticed that I am unnecessarily doing libxlDriverConfigGet
>>> twice and perhaps if a third argument is added in the future then probably
>>> consider having driver be passed as an argument?
>> Or even better have cfg as the function argument instead to allow also
>> removing
>> "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
>> the number of arguments plus allow future usage on other functions called
>> from
>> libxlBuildDomainConfig.
>
> Yep, I think that is fine. We primarily want to avoid making
> libxlBuildDomainConfig difficult to call from the unit tests. I realize we
> don't
> currently do that, but the eventual plan is to test the conversion of
> virDomainDef to libxl_domain_config. danpb did some initial work on that quite
> some time ago, see commit 5da28f24.
Ah nice to know, I wasn't aware of that work. This cover letter
(https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html) explains it
all too. What I am in this patch is clearly opposite the effort of commit
5da28f24 and a6abdbf. But now I am not sure if what I proposed in my earlier
paragrah is any different: we can probably get away with a mock of
libxlDriverConfig but not sure. In the end sounds like just adding channelDir to
the function arguments might end up being better?
Joao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] libxl: channels support
On 09/25/2016 01:13 PM, Joao Martins wrote:
> On 09/25/2016 07:55 PM, Joao Martins wrote:
>> On 09/24/2016 12:15 AM, Joao Martins wrote:
>>> On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig
>>> wrote:
On 09/22/2016 01:53 PM, Joao Martins wrote:
> [snip]
> int
> -libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
> +libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
> virDomainDefPtr def,
> libxl_ctx *ctx,
> libxl_domain_config *d_config)
> {
> +virPortAllocatorPtr graphicsports =
driver->reservedGraphicsPorts;
> +
Spurious change?
>>> This (and the other two below) was intended, as I needed
>>> channelDir. and instead of having yet another argument, I
>>> passed driver instead as graphics port was using it too.
>>>
>>> But I could use the macro directly, or add another argument if you prefer.
>> Hmm, I can just avoid passing driver and have cfg->channelDir added as an
>> argument. I just noticed that I am unnecessarily doing libxlDriverConfigGet
>> twice and perhaps if a third argument is added in the future then probably
>> consider having driver be passed as an argument?
> Or even better have cfg as the function argument instead to allow also
> removing
> "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
> the number of arguments plus allow future usage on other functions called from
> libxlBuildDomainConfig.
Yep, I think that is fine. We primarily want to avoid making
libxlBuildDomainConfig difficult to call from the unit tests. I realize we don't
currently do that, but the eventual plan is to test the conversion of
virDomainDef to libxl_domain_config. danpb did some initial work on that quite
some time ago, see commit 5da28f24.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 0/5] Add native TLS encrypted chardev TCP support
On 09/19/2016 10:39 AM, John Ferlan wrote:
> v6: http://www.redhat.com/archives/libvir-list/2016-September/msg00302.html
>
> Differences over v6 - rather than disable on specific chardev's, modify the
> code to be need to enable by setting tls='yes' for the chardev
>
ping? Any other comments for this or should I resend since it's
continuing to diverge from top of tree?
Tks -
John
> John Ferlan (5):
> domain: Add optional 'tls' attribute for TCP chardev
> conf: Introduce {default|chardev}_tls_x509_secret_uuid
> qemu: Introduce qemuDomainChardevPrivatePtr
> qemu: Add a secret object to/for a chardev tcp with secret
> qemu: Add the ability to hotplug a secret object for TCP chardev TLS
>
> docs/formatdomain.html.in | 21 +++
> docs/schemas/domaincommon.rng | 5 +
> src/conf/domain_conf.c | 51 ++--
> src/conf/domain_conf.h | 5 +-
> src/libxl/libxl_domain.c | 2 +-
> src/lxc/lxc_native.c | 2 +-
> src/qemu/libvirtd_qemu.aug | 2 +
> src/qemu/qemu.conf | 24
> src/qemu/qemu_command.c| 35 -
> src/qemu/qemu_command.h| 1 +
> src/qemu/qemu_conf.c | 22
> src/qemu/qemu_conf.h | 3 +
> src/qemu/qemu_domain.c | 143
> -
> src/qemu/qemu_domain.h | 30 -
> src/qemu/qemu_driver.c | 2 +-
> src/qemu/qemu_hotplug.c| 64 -
> src/qemu/qemu_hotplug.h| 3 +-
> src/qemu/qemu_parse_command.c | 4 +-
> src/qemu/qemu_process.c| 6 +-
> src/qemu/test_libvirtd_qemu.aug.in | 2 +
> src/vz/vz_sdk.c| 2 +-
> src/xenconfig/xen_sxpr.c | 2 +-
> tests/qemuhotplugtest.c| 2 +-
> ...emuxml2argv-serial-tcp-tlsx509-chardev-tls.args | 30 +
> ...qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml | 50 +++
> .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml| 2 +-
> ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++
> ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++
> tests/qemuxml2argvtest.c | 22
> ...muxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml | 1 +
> .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
> tests/qemuxml2xmltest.c| 1 +
> 32 files changed, 593 insertions(+), 36 deletions(-)
> create mode 100644
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.args
> create mode 100644
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml
> create mode 100644
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
> create mode 100644
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
> create mode 12
> tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] rbd: Change virStorageBackendRBDCloseRADOSConn to be static void
Since none of the callers check the status, let's just alter it from
a (global!) int to be a static void.
While we're at it - scrap the local runtime variable and just do the
math in the VIR_DEBUG directly.
Signed-off-by: John Ferlan
---
src/storage/storage_backend_rbd.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index 21693c4..4e82232 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -205,29 +205,23 @@
virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr,
return r;
}
-static int
+static void
virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr)
{
-int ret = 0;
-
if (ptr->ioctx != NULL) {
VIR_DEBUG("Closing RADOS IoCTX");
rados_ioctx_destroy(ptr->ioctx);
-ret = -1;
}
ptr->ioctx = NULL;
if (ptr->cluster != NULL) {
VIR_DEBUG("Closing RADOS connection");
rados_shutdown(ptr->cluster);
-ret = -2;
}
ptr->cluster = NULL;
-time_t runtime = time(0) - ptr->starttime;
-VIR_DEBUG("RADOS connection existed for %ld seconds", runtime);
-
-return ret;
+VIR_DEBUG("RADOS connection existed for %ld seconds",
+ time(0) - ptr->starttime);
}
static int
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] rbd: Change from static to alloc contexts
Rather than use static/stack state context pointers, let's allocate and
free the state context pointer. In doing so, we'll shrink the code a bit
since many routines perform the same initialization sequence.
Signed-off-by: John Ferlan
---
src/storage/storage_backend_rbd.c | 136 +++---
1 file changed, 69 insertions(+), 67 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index 4e82232..37375c0 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -224,6 +224,42 @@
virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr)
time(0) - ptr->starttime);
}
+
+static void
+virStorageBackendRBDFreeStateContext(virStorageBackendRBDStatePtr *ptr)
+{
+if (!*ptr)
+return;
+
+virStorageBackendRBDCloseRADOSConn(*ptr);
+
+VIR_FREE(*ptr);
+}
+
+
+static virStorageBackendRBDStatePtr
+virStorageBackendRBDAllocStateContext(virConnectPtr conn,
+ virStoragePoolObjPtr pool)
+{
+virStorageBackendRBDStatePtr ptr;
+
+if (VIR_ALLOC(ptr) < 0)
+return NULL;
+
+if (virStorageBackendRBDOpenRADOSConn(ptr, conn, &pool->def->source) < 0)
+goto error;
+
+if (virStorageBackendRBDOpenIoCTX(ptr, pool) < 0)
+goto error;
+
+return ptr;
+
+ error:
+virStorageBackendRBDFreeStateContext(&ptr);
+return NULL;
+}
+
+
static int
volStorageBackendRBDGetFeatures(rbd_image_t image,
const char *volname,
@@ -381,24 +417,19 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn,
int len = -1;
int r = 0;
char *name, *names = NULL;
-virStorageBackendRBDState ptr;
-ptr.cluster = NULL;
-ptr.ioctx = NULL;
+virStorageBackendRBDStatePtr ptr = NULL;
struct rados_cluster_stat_t clusterstat;
struct rados_pool_stat_t poolstat;
-if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, &pool->def->source) < 0)
-goto cleanup;
-
-if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0)
+if (!(ptr = virStorageBackendRBDAllocStateContext(conn, pool)))
goto cleanup;
-if ((r = rados_cluster_stat(ptr.cluster, &clusterstat)) < 0) {
+if ((r = rados_cluster_stat(ptr->cluster, &clusterstat)) < 0) {
virReportSystemError(-r, "%s", _("failed to stat the RADOS cluster"));
goto cleanup;
}
-if ((r = rados_ioctx_pool_stat(ptr.ioctx, &poolstat)) < 0) {
+if ((r = rados_ioctx_pool_stat(ptr->ioctx, &poolstat)) < 0) {
virReportSystemError(-r, _("failed to stat the RADOS pool '%s'"),
pool->def->source.name);
goto cleanup;
@@ -417,7 +448,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn,
if (VIR_ALLOC_N(names, max_size) < 0)
goto cleanup;
-len = rbd_list(ptr.ioctx, names, &max_size);
+len = rbd_list(ptr->ioctx, names, &max_size);
if (len >= 0)
break;
if (len != -ERANGE) {
@@ -443,7 +474,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn,
name += strlen(name) + 1;
-r = volStorageBackendRBDRefreshVolInfo(vol, pool, &ptr);
+r = volStorageBackendRBDRefreshVolInfo(vol, pool, ptr);
/* It could be that a volume has been deleted through a different route
* then libvirt and that will cause a -ENOENT to be returned.
@@ -476,7 +507,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn,
cleanup:
VIR_FREE(names);
-virStorageBackendRBDCloseRADOSConn(&ptr);
+virStorageBackendRBDFreeStateContext(&ptr);
return ret;
}
@@ -566,9 +597,7 @@ virStorageBackendRBDDeleteVol(virConnectPtr conn,
{
int ret = -1;
int r = 0;
-virStorageBackendRBDState ptr;
-ptr.cluster = NULL;
-ptr.ioctx = NULL;
+virStorageBackendRBDStatePtr ptr = NULL;
virCheckFlags(VIR_STORAGE_VOL_DELETE_ZEROED |
VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS, -1);
@@ -578,21 +607,18 @@ virStorageBackendRBDDeleteVol(virConnectPtr conn,
if (flags & VIR_STORAGE_VOL_DELETE_ZEROED)
VIR_WARN("%s", "This storage backend does not support zeroed removal
of volumes");
-if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, &pool->def->source) < 0)
-goto cleanup;
-
-if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0)
+if (!(ptr = virStorageBackendRBDAllocStateContext(conn, pool)))
goto cleanup;
if (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS) {
-if (virStorageBackendRBDCleanupSnapshots(ptr.ioctx, &pool->def->source,
+if (virStorageBackendRBDCleanupSnapshots(ptr->ioctx,
&pool->def->source,
vol) < 0)
goto cleanup;
}
VIR_DEBUG("Removing volume %s/%s", pool->def->source.name, vol->name);
-r = rbd_remove(ptr.ioctx, vol->name);
+r = rbd_remove(ptr->ioctx, vol->name);
if (r < 0 && (-r) !
[libvirt] [PATCH 0/3] Cleanup/unify RBD backend context setup
While working on a way to create a luks volume on an RBD backend - I figured I'd clean up the initialization of the environment a bit. Rather than a lot of copy/paste code - use a common initializer. John Ferlan (3): rbd: Change virStorageBackendRBDCloseRADOSConn to be static void rbd: Change from static to alloc contexts rbd: Move the encryption check in build src/storage/storage_backend_rbd.c | 150 +++--- 1 file changed, 73 insertions(+), 77 deletions(-) -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] rbd: Move the encryption check in build
No sense opening a connection only to fail because we don't support the
type of build being attempted.
Signed-off-by: John Ferlan
---
src/storage/storage_backend_rbd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index 37375c0..0d029c5 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -696,15 +696,15 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
goto cleanup;
}
-if (!(ptr = virStorageBackendRBDAllocStateContext(conn, pool)))
-goto cleanup;
-
if (vol->target.encryption != NULL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool does not support encrypted volumes"));
goto cleanup;
}
+if (!(ptr = virStorageBackendRBDAllocStateContext(conn, pool)))
+goto cleanup;
+
if ((r = virStorageBackendRBDCreateImage(ptr->ioctx, vol->name,
vol->target.capacity)) < 0) {
virReportSystemError(-r, _("failed to create volume '%s/%s'"),
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage_backend_rbd: remove unnessary translated message marker
On 09/23/2016 11:37 PM, Chen Hanxiao wrote: > From: Chen Hanxiao > > Remove unnessary translated message marker _() > for the VIR_WARN messages. > > Signed-off-by: Chen Hanxiao > --- > src/storage/storage_backend_rbd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > ACK and pushed. Tks, John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix Typos
On 09/24/2016 02:43 PM, Nitesh Konkar wrote: > Signed-off-by: Nitesh Konkar > --- > src/esx/esx_vi.c | 2 +- > src/libvirt-nodedev.c| 2 +- > src/openvz/openvz_conf.c | 2 +- > tools/virt-admin.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > ACK and pushed. Tks, John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh domdisplay: introduce '--all' for showing all possible graphical display
At 2016-09-22 18:43:19, "Chen Hanxiao" wrote: >From: Chen Hanxiao > >For one VM, it could have more than one graphical display. >Such as we coud add both vnc and spice display to a VM. > >This patch introduces '--all' for showing all >possible type of graphical display for a active VM. > >Signed-off-by: Chen Hanxiao >--- ping? Regards, - Chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] conf: add disk cache tuning parameters after qemu
On 26.09.2016 13:04, Peter Krempa wrote: > On Mon, Sep 26, 2016 at 12:15:42 +0300, Nikolay Shirokovskiy wrote: >> On 26.09.2016 12:01, Peter Krempa wrote: > > [...] > >> Sorry, I forget to write down the motivation. >> >> Internally qemu uses 3 orthogonal flags to configure disk cache. They >> are exposed in interface too quite long time ago (29c4e2b). All-in-one >> cache parameter that libvirt uses now do not provide all possible >> values (5 vs 8). For example absent combination of direct + no flush (no >> sync) >> is useful to speed up tests. > > Well, if there are useful combinations we can add them to the existing > cache setting. > >> So I decided to provide new possibilities just as qemu does - keep >> old parameter and use new ones to tune over. > > I don't think this is a good idea at all. It's very qemu specific and > can be mapped to existing elements. > > Peter > Well up until now libvirt cache settings are exactly the same as the qemu ones. I thought it would be inconsistent to intordocuce new name and then map internallly to qemu. Another point is that these 3 parameters are probably not that qemu specific. Nikolay -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] conf: add disk cache tuning parameters after qemu
On Mon, Sep 26, 2016 at 12:15:42 +0300, Nikolay Shirokovskiy wrote: > On 26.09.2016 12:01, Peter Krempa wrote: [...] > Sorry, I forget to write down the motivation. > > Internally qemu uses 3 orthogonal flags to configure disk cache. They > are exposed in interface too quite long time ago (29c4e2b). All-in-one > cache parameter that libvirt uses now do not provide all possible > values (5 vs 8). For example absent combination of direct + no flush (no sync) > is useful to speed up tests. Well, if there are useful combinations we can add them to the existing cache setting. > So I decided to provide new possibilities just as qemu does - keep > old parameter and use new ones to tune over. I don't think this is a good idea at all. It's very qemu specific and can be mapped to existing elements. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] conf: add disk cache tuning parameters after qemu
On 26.09.2016 12:01, Peter Krempa wrote: > On Mon, Sep 26, 2016 at 11:43:58 +0300, Nikolay Shirokovskiy wrote: >> xml example: >> >> >> > > Aren't writeback and direct mutually exclusive? > >> > > What's wrong with > > >? > > > See https://libvirt.org/formatdomain.html#elementsDisks > > > This commit message really does not describe your motivation for the > addition. Sorry, I forget to write down the motivation. Internally qemu uses 3 orthogonal flags to configure disk cache. They are exposed in interface too quite long time ago (29c4e2b). All-in-one cache parameter that libvirt uses now do not provide all possible values (5 vs 8). For example absent combination of direct + no flush (no sync) is useful to speed up tests. So I decided to provide new possibilities just as qemu does - keep old parameter and use new ones to tune over. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 11/11] conf: Skip post parse callbacks when creating copy
On Fri, Sep 23, 2016 at 15:25:04 +0200, Michal Privoznik wrote: > When creating a copy of virDomainDef we save ourselves the > trouble of writing deep-copy functions and just format and parse > back domain/device XML. However, the XML we are parsing was > already fully formatted - there is no reason to run post parse > callbacks (which fill in blanks - there are none!). > > Signed-off-by: Michal Privoznik > --- > src/conf/domain_conf.c | 6 -- > src/qemu/qemu_domain.c | 3 ++- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index bd20b74..976fe78 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -24682,7 +24682,8 @@ virDomainDefCopy(virDomainDefPtr src, > virDomainDefPtr ret; > unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE; > unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | > - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; > + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE | > + VIR_DOMAIN_DEF_PARSE_SKIP_POST_PARSE; > > if (migratable) > format_flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE | > VIR_DOMAIN_DEF_FORMAT_MIGRATABLE; ... I'd squash this to the previous patch. ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] conf: add disk cache tuning parameters after qemu
On Mon, Sep 26, 2016 at 11:43:58 +0300, Nikolay Shirokovskiy wrote: > xml example: > > > Aren't writeback and direct mutually exclusive? > What's wrong with ? See https://libvirt.org/formatdomain.html#elementsDisks This commit message really does not describe your motivation for the addition. > > Parameters semantics match corresponding qemu 'cache.*' parameters. > > -- > > I wonder should we use flush or sync name instead of qemu no_flush? > --- > docs/schemas/domaincommon.rng | 22 +++ > src/conf/domain_conf.c| 90 > +++ > src/conf/domain_conf.h| 7 > 3 files changed, 119 insertions(+) Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 10/11] domain_conf: Introduce VIR_DOMAIN_DEF_PARSE_SKIP_POST_PARSE
On Fri, Sep 23, 2016 at 15:25:03 +0200, Michal Privoznik wrote:
> This is an internal flag that prevents our two entry points to
> XML parsing (virDomainDefParse and virDomainDeviceDefParse) from
> running post parse callbacks. This is expected to be used in
> cases when we already have full domain/device XML and we are just
> parsing it back (i.e. virDomainDefCopy or virDomainDeviceDefCopy)
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/domain_conf.c | 8
> src/conf/domain_conf.h | 2 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b11a296..bd20b74 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4428,6 +4428,10 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> {
> int ret;
>
> +/* fill in configuration only in certain places */
> +if (flags & VIR_DOMAIN_DEF_PARSE_SKIP_POST_PARSE)
> +return 0;
> +
> if (xmlopt->config.devicesPostParseCallback) {
> ret = xmlopt->config.devicesPostParseCallback(dev, def, caps, flags,
>xmlopt->config.priv,
> @@ -4577,6 +4581,10 @@ virDomainDefPostParse(virDomainDefPtr def,
> .parseOpaque = parseOpaque,
> };
>
> +/* fill in configuration only in certain places */
> +if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_POST_PARSE)
> +return 0;
> +
The comments are rather useless.
ACK if you remove them.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu_alias:Set dimm alias name to dimmX where X=slot number.
On Sun, Sep 25, 2016 at 23:32:05 +0530, Nitesh Konkar wrote:
> When we hotplug a memory module, qemu will assign
> the smallest slot available. At the time of setting
> the alias we find the smallest available slot and
> assign alias name as dimmX where X=slot number.
This commit message doesn't really describe why this is necessary. The
problem with qemu is that the migration stream references the memory
blocks by name (which is supplied by libvirt) rather than by the order
of those. This means that we must keep them stable accross migrations.
With the current code that is assigning aliases for memory backend
objects this won't happen and since qemu is treating the memory object
links differently migration does not work in such case.
This means that the requirement is to keep the slot number consistent
with the alias.
To achieve this the below code is not really necessary as once the VM is
started the slot numbers are populated (and thankfully correctly since
qemu numbers the slots starting from 0).
The code below is based on parsing aliases which is not necessary since
you already have the slot numbers available.
The general idea is to do slot number alocation for the memory modules
beforehand so that we are 100% sure which slot number we will get and
then generate alias based on the slot number. I'm not sure though
whether qemu accepts a partial address information consisting of the
slot number only and I did not get around to check it yet.
>
> Signed-off-by: Nitesh Konkar
> ---
> src/qemu/qemu_alias.c | 51
> +--
> 1 file changed, 41 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index 0102c96..23ffdfd 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -28,6 +28,7 @@
> #include "virlog.h"
> #include "virstring.h"
> #include "network/bridge_driver.h"
> +#include "qemu_process.h"
Why do you need this? The code below doesn't seem to use it in any way.
>
> #define QEMU_DRIVE_HOST_PREFIX "drive-"
>
> @@ -331,21 +332,47 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def,
> return 0;
> }
>
> +/* Find the smallest available slot. */
> +static int
> +qemuGetSmallestSlotIdx(virDomainDefPtr def)
> +{
> +size_t i, j;
Please don't declare multiple variables on a line.
> +int minidx = 0, idx;
Also declaring multiple on same line and initializing some of them is
even worse.
> +
> +for (i = 0; i < def->nmems; i++) {
> +if ((idx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm"))
> != i) {
[1] You've parsed the alias here once now.
> +for (j = 0; j < def->nmems; j++) {
> +if ((idx = qemuDomainDeviceAliasIndex(&def->mems[j]->info,
> "dimm")) == i)
> +break;
> +}
> +
> +if (j >= def->nmems) {
> +minidx = i;
> +break;
> +}
This is a rather weird algorithm.
> +}
> +
> +if ((idx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm"))
> >= minidx)
You are doing the same as in [1]. Seems like a waste.
> +minidx = idx + 1;
> +}
> +
> +return minidx;
> +}
I don't see a reason to have the function above if we switch to
pre-allocating the numbers properly in the first place. Also then it
will be possible to find the index just by looking at the slot number
rather than parsing the alias over and over.
If you're not into making sure that qemu accepts slots and doing the
slot allocation I can do that since I was up to fix this problem
anyways.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/11] qemuDomainDefAssignAddresses: Fetch caps from domain object
On Fri, Sep 23, 2016 at 15:25:02 +0200, Michal Privoznik wrote:
> Just like we did two commits ago, don't try to fetch capabilities
> for non-existing binary. Re-use the ones we have for running
> domain.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_domain.c | 16 ++--
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 70fac56..2b24c01 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2774,17 +2774,21 @@ qemuDomainDefAssignAddresses(virDomainDef *def,
> virCapsPtr caps,
> unsigned int parseFlags ATTRIBUTE_UNUSED,
> void *opaque,
> - void *parseOpaque ATTRIBUTE_UNUSED)
> + void *parseOpaque)
> {
> virQEMUDriverPtr driver = opaque;
> -virQEMUCapsPtr qemuCaps = NULL;
> +virQEMUCapsPtr qemuCaps = parseOpaque;
> int ret = -1;
> bool newDomain = parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
>
> -if (!(qemuCaps = virQEMUCapsCacheLookup(caps,
> -driver->qemuCapsCache,
> -def->emulator)))
> -goto cleanup;
> +if (qemuCaps) {
> +virObjectRef(qemuCaps);
> +} else {
> +if (!(qemuCaps = virQEMUCapsCacheLookup(caps,
> +driver->qemuCapsCache,
> +def->emulator)))
> +goto cleanup;
> +}
>
> if (qemuDomainAssignAddresses(def, qemuCaps, NULL, newDomain) < 0)
> goto cleanup;
Do we really need to have a separate patch for a single change in
several functions?
ACK
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 08/11] conf: Extend virDomainDefAssignAddressesCallback for parseOpaque
On Fri, Sep 23, 2016 at 15:25:01 +0200, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik
> ---
> src/conf/domain_conf.c | 3 ++-
> src/conf/domain_conf.h | 8 ++--
> src/qemu/qemu_domain.c | 3 ++-
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d205222..b11a296 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4604,7 +4604,8 @@ virDomainDefPostParse(virDomainDefPtr def,
>
> if (xmlopt->config.assignAddressesCallback) {
> ret = xmlopt->config.assignAddressesCallback(def, caps, parseFlags,
> - xmlopt->config.priv);
> + xmlopt->config.priv,
> + parseOpaque);
> if (ret < 0)
> return ret;
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 37ed6f5..39d7d8c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2382,11 +2382,15 @@ typedef int
> (*virDomainDeviceDefPostParseCallback)(virDomainDeviceDefPtr dev,
> void *opaque,
> void *parseOpaque);
> /* Drive callback for assigning device addresses, called at the end
> - * of parsing, after all defaults and implicit devices have been added. */
> + * of parsing, after all defaults and implicit devices have been added.
> + * @parseOpaque is opaque data passed by virDomainDefParse* caller,
> + * @opqaue is opaque data set by driver (usually pointer to driver
s/opqaue/opaque/
> + * private data). */
> typedef int (*virDomainDefAssignAddressesCallback)(virDomainDef *def,
> virCapsPtr caps,
> unsigned int parseFlags,
> - void *opaque);
> + void *opaque,
> + void *parseOpaque);
>
> /* Called in appropriate places where the domain conf parser can return
> failure
> * for configurations that were previously accepted. This shall not modify
> the
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 99baf5c..70fac56 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2773,7 +2773,8 @@ static int
> qemuDomainDefAssignAddresses(virDomainDef *def,
> virCapsPtr caps,
> unsigned int parseFlags ATTRIBUTE_UNUSED,
> - void *opaque)
> + void *opaque,
> + void *parseOpaque ATTRIBUTE_UNUSED)
> {
> virQEMUDriverPtr driver = opaque;
> virQEMUCapsPtr qemuCaps = NULL;
ACK
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [[PATCH v2] 0/4] try harder to get dest qemu errors on migation
ping On 12.09.2016 17:34, Nikolay Shirokovskiy wrote: > Hi, all. > > In case migration fails due to destination qemu exits unexpectedly user > recevies the qemu log in the error message. Unfortunately log is truncated and > the most interesting part is missed (below is the example of such a log [1]). > > Actually for the most cases the first patch will be enough to fix the issue. > Originally I thought the problem is qemu logging and reading the log are not > in > sync (which is true) so I tried to fix it as well in the next patches. > > * diff from v1: > > 1. split changes to libvirtd and virtlogd to different patches > 2. split virtlogd patch further > 3. simplify handling eofs and hangups in draining function > > [1] log example: > > CPU Reset (CPU 0) > EAX= EBX= ECX= EDX= > ESI= EDI= EBP= ESP= > EIP= EFL= [---] CPL=0 II=0 A20=0 SMM=0 HLT=0 > ES = > CS = > SS = > DS = > FS = > GS = > LDT= > TR = > GDT= > IDT= > CR0= CR2= CR3= CR4= > DR0= DR1= DR2= > DR3= > DR6= DR7= > CCS= CCD= CCO=DYNAMIC > EFER= > FCW= FSW= [ST=0] FTW=ff MXCSR= > FPR0= FPR1= > FPR2= FPR3= > FPR4= FPR5= > FPR6= FPR7= > XMM00= XMM01= > XMM02= XMM03= > XMM04= XMM05= > XMM06= XMM07= > CPU Reset (CPU 1) > EAX= EBX= ECX= EDX=000206a1 > ESI= EDI= EBP= ESP= > EIP=fff0 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0 > ES = 9300 > CS =f000 9b00 > SS = 9300 > DS = 9300 > FS = 9300 > GS = 9300 > LDT= 8200 > TR = 8b00 > GDT= > IDT= > CR0=6010 CR2= CR3= CR4= > DR0= DR1= DR2= > DR3= > DR6=0ff0 DR7=0400 > CCS= CCD= CCO=DYNAMIC > EFER= > FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80 > FPR0= FPR1= > FPR2= FPR3= > FPR4= FPR5= > FPR6= FPR7= > XMM00= XMM01= > XMM02= XMM03= > XMM04= XMM05= > XMM06= XMM07=000 > qemu: terminating on signal 15 from pid 168133 > > Nikolay Shirokovskiy (4): > util: remove 1k limit for error messages > virtlogd: stop reading on EOF instead of hangup > virtlogd: add flag to wait for log end on read > qemu: if virtlogd is used then read log tail correctly > > src/logging/log_handler.c | 46 > -- > src/logging/log_protocol.x | 5 + > src/qemu/qemu_domain.c | 7 ++- > src/qemu/qemu_domain.h | 1 + > src/qemu/qemu_process.c| 2 ++ > src/util/virerror.c| 9 - > 6 files changed, 58 insertions(+), 12 deletions(-) > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/11] qemuDomainDeviceDefPostParse: Fetch caps from domain object
On Mon, Sep 26, 2016 at 10:45:51 +0200, Jiri Denemark wrote:
> On Fri, Sep 23, 2016 at 15:25:00 +0200, Michal Privoznik wrote:
> > Just like we did two commits ago, don't try to fetch capabilities
> > for non-existing binary. Re-use the ones we have for running
> > domain.
> >
> > Signed-off-by: Michal Privoznik
> > ---
> > src/qemu/qemu_domain.c | 12
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 1f5ef92..99baf5c 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -2562,15 +2562,19 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr
> > dev,
> > virCapsPtr caps,
> > unsigned int parseFlags,
> > void *opaque,
> > - void *parseOpaque ATTRIBUTE_UNUSED)
> > + void *parseOpaque)
> > {
> > virQEMUDriverPtr driver = opaque;
> > -virQEMUCapsPtr qemuCaps = NULL;
> > +virQEMUCapsPtr qemuCaps = parseOpaque;
> > virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> > int ret = -1;
> >
> > -qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache,
> > - def->emulator);
> > +if (qemuCaps) {
> > +virObjectRef(qemuCaps);
> > +} else {
> > +qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache,
> > + def->emulator);
> > +}
> >
> > if (dev->type == VIR_DOMAIN_DEVICE_NET &&
> > dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
>
> Similar to 5/11, can't you just squash them? (After squashing 4/11 and
> 6/11, of course).
Or rather just reordering them. I thought 4/11 was similar to 6/11...
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/11] qemuDomainDeviceDefPostParse: Fetch caps from domain object
On Fri, Sep 23, 2016 at 15:25:00 +0200, Michal Privoznik wrote:
> Just like we did two commits ago, don't try to fetch capabilities
> for non-existing binary. Re-use the ones we have for running
> domain.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_domain.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 1f5ef92..99baf5c 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2562,15 +2562,19 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr
> dev,
> virCapsPtr caps,
> unsigned int parseFlags,
> void *opaque,
> - void *parseOpaque ATTRIBUTE_UNUSED)
> + void *parseOpaque)
> {
> virQEMUDriverPtr driver = opaque;
> -virQEMUCapsPtr qemuCaps = NULL;
> +virQEMUCapsPtr qemuCaps = parseOpaque;
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> int ret = -1;
>
> -qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache,
> - def->emulator);
> +if (qemuCaps) {
> +virObjectRef(qemuCaps);
> +} else {
> +qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache,
> + def->emulator);
> +}
>
> if (dev->type == VIR_DOMAIN_DEVICE_NET &&
> dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
Similar to 5/11, can't you just squash them? (After squashing 4/11 and
6/11, of course).
ACK
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] conf: add disk cache tuning parameters after qemu
xml example:
Parameters semantics match corresponding qemu 'cache.*' parameters.
--
I wonder should we use flush or sync name instead of qemu no_flush?
---
docs/schemas/domaincommon.rng | 22 +++
src/conf/domain_conf.c| 90 +++
src/conf/domain_conf.h| 7
3 files changed, 119 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 95c7882..17e4ac9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1208,6 +1208,9 @@
+
+
+
@@ -1597,6 +1600,25 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dd34cec..092696c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7405,6 +7405,52 @@ virDomainDiskDefParseValidate(const virDomainDiskDef
*def)
return 0;
}
+static int
+virDomainDiskCachetuneDefFormat(virBufferPtr buf,
+virDomainDiskDefPtr def)
+{
+const char *writeback =
virTristateSwitchTypeToString(def->cachetune.writeback);
+const char *direct = virTristateSwitchTypeToString(def->cachetune.direct);
+const char *no_flush =
virTristateSwitchTypeToString(def->cachetune.no_flush);
+
+if (!writeback) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk cachetune writeback mode %d"),
+ def->cachetune.writeback);
+return -1;
+}
+
+if (!direct) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk cachetune direct mode %d"),
+ def->cachetune.direct);
+return -1;
+}
+
+if (!no_flush) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk cachetune no_flush mode %d"),
+ def->cachetune.no_flush);
+return -1;
+}
+
+if (def->cachetune.writeback ||
+def->cachetune.direct ||
+def->cachetune.no_flush) {
+virBufferAddLit(buf, "cachetune.writeback)
+virBufferAsprintf(buf, " writeback='%s'", writeback);
+if (def->cachetune.direct)
+virBufferAsprintf(buf, " direct='%s'", direct);
+if (def->cachetune.no_flush)
+virBufferAsprintf(buf, " no_flush='%s'", no_flush);
+
+virBufferAddLit(buf, "/>\n");
+}
+
+return 0;
+}
static int
virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def,
@@ -7521,6 +7567,45 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def,
return ret;
}
+static int
+virDomainDiskDefCachetuneParse(virDomainDiskDefPtr def,
+ xmlNodePtr cur,
+ xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED)
+{
+char *tmp = NULL;
+int ret = -1;
+
+if ((tmp = virXMLPropString(cur, "writeback")) &&
+(def->cachetune.writeback = virTristateSwitchTypeFromString(tmp)) < 0)
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown disk cache writeback setting '%s'"), tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);
+
+if ((tmp = virXMLPropString(cur, "direct")) &&
+(def->cachetune.direct = virTristateSwitchTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown disk cache direct setting '%s'"), tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);
+
+if ((tmp = virXMLPropString(cur, "no_flush")) &&
+(def->cachetune.no_flush = virTristateSwitchTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown disk cache no_flush setting '%s'"), tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(tmp);
+
+return ret;
+}
#define VENDOR_LEN 8
#define PRODUCT_LEN 16
@@ -7738,6 +7823,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
} else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
/* boot is parsed as part of virDomainDeviceInfoParseXML */
+} else if (xmlStrEqual(cur->name, BAD_CAST "cachetune")) {
+if (virDomainDiskDefCachetuneParse(def, cur, ctxt) < 0)
+goto error;
}
}
@@ -20182,6 +20270,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
virDomainDiskGeometryDefFormat(buf, def);
virDomainDiskBlockIoDefFormat(buf, def);
+if (virDomainDiskCachetuneDefFormat(buf, def) < 0)
+return -1;
/* For now, mirroring is currently output-only: we only output it
* for live domains, therefore we ignore it on input except for
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ce90c27.
[libvirt] [PATCH 0/2] support qemu drive cache.* parameters
Nikolay Shirokovskiy (2): conf: add disk cache tuning parameters after qemu qemu: support in domain disk xml .gnulib| 2 +- docs/schemas/domaincommon.rng | 22 ++ src/conf/domain_conf.c | 90 ++ src/conf/domain_conf.h | 7 ++ src/qemu/qemu_capabilities.c | 12 ++- src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c| 30 tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 3 + .../caps_2.6.0-gicv2.aarch64.xml | 3 + .../caps_2.6.0-gicv3.aarch64.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 3 + 17 files changed, 194 insertions(+), 2 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: support in domain disk xml
Qemu supports cache.* parameters since 1.6 version. However introspection
for their presence is available in later versions only.
---
I don't bothered to figuring out from what version introspection is
possible thus don't touch tests/qemucapabilitiesdata/caps_*.replies.
Hoever it would be nice to know this version. In this case version
checking can have upper bound and can be eventually dropped from
the code when libvirt drops support of this version.
---
.gnulib| 2 +-
src/qemu/qemu_capabilities.c | 12 -
src/qemu/qemu_capabilities.h | 3 +++
src/qemu/qemu_command.c| 30 ++
tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 3 +++
tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 3 +++
tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 3 +++
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 +++
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 3 +++
.../caps_2.6.0-gicv2.aarch64.xml | 3 +++
.../caps_2.6.0-gicv3.aarch64.xml | 3 +++
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 3 +++
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 3 +++
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 3 +++
14 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/.gnulib b/.gnulib
index e89b4a7..a2a3943 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit e89b4a7aefce9cb02963920712ba7cdd13641644
+Subproject commit a2a39436b65f329630df4a93ec4e30aeae403c54
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d859c4..8e54f99 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -344,6 +344,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"query-hotpluggable-cpus",
"virtio-net.rx_queue_size", /* 235 */
+ "drive-cache.writeback",
+ "drive-cache.direct",
+ "drive-cache.no_flush",
);
@@ -2836,6 +2839,9 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
{ "drive", "discard", QEMU_CAPS_DRIVE_DISCARD },
{ "drive", "detect-zeroes", QEMU_CAPS_DRIVE_DETECT_ZEROES },
+{ "drive", "cache.writeback", QEMU_CAPS_DRIVE_CACHE_WRITEBACK },
+{ "drive", "cache.direct", QEMU_CAPS_DRIVE_CACHE_DIRECT },
+{ "drive", "cache.no-flush", QEMU_CAPS_DRIVE_CACHE_NO_FLUSH },
{ "realtime", "mlock", QEMU_CAPS_REALTIME_MLOCK },
{ "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT },
{ "boot-opts", "reboot-timeout", QEMU_CAPS_REBOOT_TIMEOUT },
@@ -3756,8 +3762,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (qemuCaps->version >= 1005000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT);
-if (qemuCaps->version >= 1006000)
+if (qemuCaps->version >= 1006000) {
virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_WRITEBACK);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_DIRECT);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_NO_FLUSH);
+}
/* vmport option is supported v2.2.0 onwards */
if (qemuCaps->version >= 2002000)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index ba0ef48..01d37a9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -378,6 +378,9 @@ typedef enum {
/* 235 */
QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */
+QEMU_CAPS_DRIVE_CACHE_WRITEBACK, /* cache.writeback */
+QEMU_CAPS_DRIVE_CACHE_DIRECT, /* cache.direct */
+QEMU_CAPS_DRIVE_CACHE_NO_FLUSH, /* cache.no-flush */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f24a98b..347f902 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1635,6 +1635,36 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
virBufferAddLit(&opt, ",cache=none");
}
+if (disk->cachetune.writeback > 0) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_WRITEBACK)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disk cache writeback parameter is not "
+ "supported by this QEMU"));
+}
+virBufferAsprintf(&opt, ",cache.writeback=%s",
+
virTristateSwitchTypeToString(disk->cachetune.writeback));
+}
+
+if (disk->cachetune.direct > 0) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_DIRECT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disk cache direct parameter is not "
+ "supported by this QEMU"));
+}
+virBufferAsprintf(&opt,
Re: [libvirt] [PATCH v2 06/11] conf: Extend virDomainDeviceDefPostParse for parseOpaque
On Fri, Sep 23, 2016 at 15:24:59 +0200, Michal Privoznik wrote: > Just like virDomainDefPostParseCallback has gained new > parseOpaque argument, we need to follow the logic with > virDomainDeviceDefPostParse. > ... > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index a6554f5..37ed6f5 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2371,12 +2371,16 @@ typedef int > (*virDomainDefPostParseCallback)(virDomainDefPtr def, > void *opaque, > void *parseOpaque); > /* Called once per device, for adjusting per-device settings while > - * leaving the overall domain otherwise unchanged. */ > + * leaving the overall domain otherwise unchanged. > + * @parseOpaque is opaque data passed by virDomainDefParse* caller, > + * @opqaue is opaque data set by driver (usually pointer to driver s/opqaue/opaque/ > + * private data). */ > typedef int (*virDomainDeviceDefPostParseCallback)(virDomainDeviceDefPtr dev, > const virDomainDef *def, > virCapsPtr caps, > unsigned int parseFlags, > - void *opaque); > + void *opaque, > + void *parseOpaque); > /* Drive callback for assigning device addresses, called at the end > * of parsing, after all defaults and implicit devices have been added. */ > typedef int (*virDomainDefAssignAddressesCallback)(virDomainDef *def, ... ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 05/11] qemuDomainDefPostParse: Fetch qemuCaps from domain object
On Fri, Sep 23, 2016 at 15:24:58 +0200, Michal Privoznik wrote:
> We can't rely on def->emulator path. It may be provided by user
> as we give them opportunity to provide their own XML for
> migration. Therefore the path may point to just whatever binary
> (or even to a non-existent file). Moreover, this path is meant
> for destination, but the capabilities lookup is done on source.
> What we can do is to assume same capabilities for post parse
> callbacks as the running domain has. They will be used just to
> add some default models/controllers/devices/... anyway.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_domain.c| 16 ++--
> src/qemu/qemu_migration.c | 2 +-
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 97f8993..ea88f5e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2327,11 +2327,11 @@ qemuDomainDefPostParse(virDomainDefPtr def,
> virCapsPtr caps,
> unsigned int parseFlags,
> void *opaque,
> - void *parseOpaque ATTRIBUTE_UNUSED)
> + void *parseOpaque)
> {
> virQEMUDriverPtr driver = opaque;
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> -virQEMUCapsPtr qemuCaps = NULL;
> +virQEMUCapsPtr qemuCaps = parseOpaque;
> int ret = -1;
>
> if (def->os.bootloader || def->os.bootloaderArgs) {
> @@ -2360,10 +2360,14 @@ qemuDomainDefPostParse(virDomainDefPtr def,
> !(def->emulator = virDomainDefGetDefaultEmulator(def, caps)))
> goto cleanup;
>
> -if (!(qemuCaps = virQEMUCapsCacheLookup(caps,
> -driver->qemuCapsCache,
> -def->emulator)))
> -goto cleanup;
> +if (qemuCaps) {
> +virObjectRef(qemuCaps);
> +} else {
> +if (!(qemuCaps = virQEMUCapsCacheLookup(caps,
> +driver->qemuCapsCache,
> +def->emulator)))
> +goto cleanup;
> +}
This could be simplified a bit as
if (!virObjectRef(qemuCaps) &&
!(qemuCaps = virQEMUCapsCacheLookup(caps,
driver->qemuCapsCache,
def->emulator)))
goto cleanup;
But not a big deal.
ACK
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/11] virDomainDefCopy: Introduce @parseOpaque argument
On Fri, Sep 23, 2016 at 15:24:57 +0200, Michal Privoznik wrote: > We want to by pass the proper opaque pointer instead of NULL to Copy & paste, isn't it? :-) > virDomainDefParseString. ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/11] virDomainDefParse{File, String}: Introduce @parseOpaque argument
On Fri, Sep 23, 2016 at 15:24:56 +0200, Michal Privoznik wrote: > We want to by pass the proper opaque pointer instead of NULL to s/by // ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainUpdateDeviceConfig: Allow full disk update
On Fri, Sep 23, 2016 at 05:42:41PM -0400, John Ferlan wrote: On 09/02/2016 07:41 AM, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1368417 So far, when it comes to 'virsh update-device --config' of disks we are limiting ourselves for just the disk source update and just for CDROMs and floppies. This makes no sense. Especially if you look around and see that we already allow full update to graphics and net devices. So let's just take whatever XML user wants to have there and replace our internal definition with it. Signed-off-by: Michal Privoznik --- src/qemu/qemu_driver.c | 26 +++--- 1 file changed, 7 insertions(+), 19 deletions(-) Crazy this has been this way since 0.9.0 (commit id 'f37c29c8a'). Although this patch certainly does more than just ensure the startupPolicy adjustment works for --config as well as --live (which is the basis of the bug). I would think to "just fix" the bug/issue "DISK" could be added to that negative if condition, which leaves just LUN as a type that wouldn't be updateable (since startupPolicy is updated in the subsequent code). I guess the question is - is there something in the DISK or LUN specific usages that shouldn't be just updated or something that could be lost if we simply delete the old one without copying "something" in (_virDomainDiskDef is used for a lot and especially that private piece which could be of concern). The text in the last paragraph of virDomainUpdateDeviceFlags just doesn't give me enough of a hint why FLOPPY and CDROM were specifically singled out. It has been that way because we can't update everything LIVE. And CONFIG just followed (or was left to rot in the fiery pits of I'm-not-touching-this-code land). Basically, update with CONFIG is the same thing that you would do with 'virsh edit', just for a particular device. And we allow everything to be edited and saved there. So ther eis no reason for restricting anything here in this function, unless there's some requirement in the callers. And since there is one caller that uses this to modify a temporary definition (and updates the domain only after everything else succeeded) I see no reason why this couldn't go in. So from me it's an ACK. Feel free to chime in if you have other ideas. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
