May 2011 - Devel - Libvirt List Archives

[libvirt] [PATCH 3/9] add DHCP snooping support to nwfilter

by David L Stevens

This patch changes rules of the form: if ! addr drop accept to: if addr return ... drop The patch adds a "mac" chain to do a mac address list and separates the "arp" chain into separate "arpmac" and "arpip" chains that can check multiple MAC or IP addresses in any combination. This patch itself does not support multiple addresses via the MAC and IP variables, but only changes the form of the rules to allow multiple addresses in the future. Signed-off-by: David L Stevens <dlstevens(a)us.ibm.com> diff --git a/examples/xml/nwfilter/Makefile.am b/examples/xml/nwfilter/Makefile.am index 439e7b8..8ef9a71 100644 --- a/examples/xml/nwfilter/Makefile.am +++ b/examples/xml/nwfilter/Makefile.am @@ -7,6 +7,8 @@ FILTERS = \ allow-ipv4.xml \ clean-traffic.xml \ no-arp-spoofing.xml \ + no-arpmac-spoofing.xml \ + no-arpip-spoofing.xml \ no-ip-multicast.xml \ no-ip-spoofing.xml \ no-mac-broadcast.xml \ diff --git a/examples/xml/nwfilter/allow-arp.xml b/examples/xml/nwfilter/allow-arp.xml index 63a92b2..006bb54 100644 --- a/examples/xml/nwfilter/allow-arp.xml +++ b/examples/xml/nwfilter/allow-arp.xml @@ -1,3 +1,6 @@ -<filter name='allow-arp' chain='arp'> +<filter name='allow-arp' chain='arpmac'> + <rule direction='inout' action='accept'/> +</filter> +<filter name='allow-arp' chain='arpip'> <rule direction='inout' action='accept'/> </filter> diff --git a/examples/xml/nwfilter/clean-traffic.xml b/examples/xml/nwfilter/clean-traffic.xml index 40f0ecb..9cee799 100644 --- a/examples/xml/nwfilter/clean-traffic.xml +++ b/examples/xml/nwfilter/clean-traffic.xml @@ -11,10 +11,10 @@  <filterref filter='no-arp-spoofing'/> -  - <filterref filter='no-other-l2-traffic'/> -  <filterref filter='qemu-announce-self'/> +  + <filterref filter='no-other-l2-traffic'/> + </filter> diff --git a/examples/xml/nwfilter/no-arp-spoofing.xml b/examples/xml/nwfilter/no-arp-spoofing.xml index fdd4e60..1979b20 100644 --- a/examples/xml/nwfilter/no-arp-spoofing.xml +++ b/examples/xml/nwfilter/no-arp-spoofing.xml @@ -1,17 +1,4 @@ -<filter name='no-arp-spoofing' chain='arp'> - <uuid>f88f1932-debf-4aa1-9fbe-f10d3aa4bc95</uuid> - <rule action='drop' direction='out' priority='300' > - <mac match='no' srcmacaddr='$MAC'/> - </rule> - -  -  - <rule action='drop' direction='out' priority='350' > - <arp match='no' arpsrcmacaddr='$MAC'/> - </rule> - <rule action='drop' direction='out' priority='400' > - <arp match='no' arpsrcipaddr='$IP' /> - </rule> -  - <rule action='drop' direction='out' priority='1000' /> +<filter name='no-arp-spoofing'> + <filterref filter='no-arpmac-spoofing' /> + <filterref filter='no-arpip-spoofing' /> </filter> diff --git a/examples/xml/nwfilter/no-arpip-spoofing.xml b/examples/xml/nwfilter/no-arpip-spoofing.xml new file mode 100644 index 0000000..ee42d40 --- /dev/null +++ b/examples/xml/nwfilter/no-arpip-spoofing.xml @@ -0,0 +1,12 @@ +<filter name='no-arpip-spoofing' chain='arpip'> +  +  + <rule action='return' direction='out' priority='400' > + <arp match='yes' arpsrcipaddr='$IP' /> + </rule> + <rule action='return' direction='out' priority='410' > + <arp match='yes' arpsrcipaddr='0.0.0.0' /> + </rule> +  + <rule action='drop' direction='out' priority='1000' /> +</filter> diff --git a/examples/xml/nwfilter/no-arpmac-spoofing.xml b/examples/xml/nwfilter/no-arpmac-spoofing.xml new file mode 100644 index 0000000..90499d3 --- /dev/null +++ b/examples/xml/nwfilter/no-arpmac-spoofing.xml @@ -0,0 +1,7 @@ +<filter name='no-arpmac-spoofing' chain='arpmac'> + <rule action='return' direction='out' priority='350' > + <arp match='yes' arpsrcmacaddr='$MAC'/> + </rule> +  + <rule action='drop' direction='out' priority='1000' /> +</filter> diff --git a/examples/xml/nwfilter/no-ip-spoofing.xml b/examples/xml/nwfilter/no-ip-spoofing.xml index b8c94c8..84e8a5e 100644 --- a/examples/xml/nwfilter/no-ip-spoofing.xml +++ b/examples/xml/nwfilter/no-ip-spoofing.xml @@ -1,7 +1,9 @@ <filter name='no-ip-spoofing' chain='ipv4'>  - <rule action='drop' direction='out'> - <ip match='no' srcipaddr='$IP' /> + <rule action='return' direction='out'> + <ip match='yes' srcipaddr='$IP' /> </rule> +  + <rule action='drop' direction='out' /> </filter> diff --git a/examples/xml/nwfilter/no-mac-spoofing.xml b/examples/xml/nwfilter/no-mac-spoofing.xml index f210623..aee56c7 100644 --- a/examples/xml/nwfilter/no-mac-spoofing.xml +++ b/examples/xml/nwfilter/no-mac-spoofing.xml @@ -1,5 +1,9 @@ -<filter name='no-mac-spoofing' chain='ipv4'> - <rule action='drop' direction='out' priority='10'> - <mac match='no' srcmacaddr='$MAC' /> - </rule> +<filter name='no-mac-spoofing' chain='mac'> +  +  + <rule action='return' direction='out' priority='350' > + <mac match='yes' srcmacaddr='$MAC'/> + </rule> +  + <rule action='drop' direction='out' priority='1000' /> </filter> diff --git a/examples/xml/nwfilter/no-other-l2-traffic.xml b/examples/xml/nwfilter/no-other-l2-traffic.xml index 8bad86e..0501b1a 100644 --- a/examples/xml/nwfilter/no-other-l2-traffic.xml +++ b/examples/xml/nwfilter/no-other-l2-traffic.xml @@ -1,7 +1,12 @@ -<filter name='no-other-l2-traffic'> +<filter name='no-other-l2-traffic' chain='root'> -  - <rule action='drop' direction='inout' priority='1000'/> +  + <rule action='accept' direction='inout'> + <mac protocolid='0x800' /> + </rule> + <rule action='accept' direction='inout'> + <mac protocolid='0x806' /> + </rule> + <rule action='drop' direction='inout' priority='1000' /> </filter> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index c5705c1..df1a012 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -82,7 +82,9 @@ VIR_ENUM_IMPL(virNWFilterEbtablesTable, VIR_NWFILTER_EBTABLES_TABLE_LAST, VIR_ENUM_IMPL(virNWFilterChainSuffix, VIR_NWFILTER_CHAINSUFFIX_LAST, "root", - "arp", + "mac", + "arpmac", + "arpip", "rarp", "ipv4", "ipv6"); diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index ef60b6b..4d60751 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -425,7 +425,9 @@ struct _virNWFilterEntry { enum virNWFilterChainSuffixType { VIR_NWFILTER_CHAINSUFFIX_ROOT = 0, - VIR_NWFILTER_CHAINSUFFIX_ARP, + VIR_NWFILTER_CHAINSUFFIX_MAC, + VIR_NWFILTER_CHAINSUFFIX_ARPMAC, + VIR_NWFILTER_CHAINSUFFIX_ARPIP, VIR_NWFILTER_CHAINSUFFIX_RARP, VIR_NWFILTER_CHAINSUFFIX_IPv4, VIR_NWFILTER_CHAINSUFFIX_IPv6, diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 39bd4a5..fa6f719 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -129,20 +129,24 @@ struct ushort_map { enum l3_proto_idx { - L3_PROTO_IPV4_IDX = 0, - L3_PROTO_IPV6_IDX, - L3_PROTO_ARP_IDX, + L3_PROTO_MAC_IDX = 0, + L3_PROTO_ARPMAC_IDX, + L3_PROTO_ARPIP_IDX, L3_PROTO_RARP_IDX, + L3_PROTO_IPV4_IDX, + L3_PROTO_IPV6_IDX, L3_PROTO_LAST_IDX }; #define USHORTMAP_ENTRY_IDX(IDX, ATT, VAL) [IDX] = { .attr = ATT, .val = VAL } static const struct ushort_map l3_protocols[] = { - USHORTMAP_ENTRY_IDX(L3_PROTO_IPV4_IDX, ETHERTYPE_IP , "ipv4"), - USHORTMAP_ENTRY_IDX(L3_PROTO_IPV6_IDX, ETHERTYPE_IPV6 , "ipv6"), - USHORTMAP_ENTRY_IDX(L3_PROTO_ARP_IDX , ETHERTYPE_ARP , "arp"), - USHORTMAP_ENTRY_IDX(L3_PROTO_RARP_IDX, ETHERTYPE_REVARP, "rarp"), + USHORTMAP_ENTRY_IDX(L3_PROTO_MAC_IDX, 0 , "mac"), + USHORTMAP_ENTRY_IDX(L3_PROTO_IPV4_IDX, ETHERTYPE_IP , "ipv4"), + USHORTMAP_ENTRY_IDX(L3_PROTO_IPV6_IDX, ETHERTYPE_IPV6 , "ipv6"), + USHORTMAP_ENTRY_IDX(L3_PROTO_ARPMAC_IDX,ETHERTYPE_ARP , "arpmac"), + USHORTMAP_ENTRY_IDX(L3_PROTO_ARPIP_IDX, ETHERTYPE_ARP , "arpip"), + USHORTMAP_ENTRY_IDX(L3_PROTO_RARP_IDX, ETHERTYPE_REVARP, "rarp"), USHORTMAP_ENTRY_IDX(L3_PROTO_LAST_IDX, 0 , NULL), }; @@ -1946,7 +1950,7 @@ ebtablesCreateRuleInstance(char chainPrefix, virBufferVSprintf(&buf, " -p 0x%x", (rule->prtclType == VIR_NWFILTER_RULE_PROTOCOL_ARP) - ? l3_protocols[L3_PROTO_ARP_IDX].attr + ? l3_protocols[L3_PROTO_ARPMAC_IDX].attr : l3_protocols[L3_PROTO_RARP_IDX].attr); if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataHWType)) { @@ -2767,15 +2771,22 @@ ebtablesCreateTmpSubChain(virBufferPtr buf, char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP : CHAINPREFIX_HOST_OUT_TEMP; + char protostr[16]; PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname); PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val); + if (l3_protocols[protoidx].attr) + snprintf(protostr, sizeof(protostr), "-p 0x%04x ", + l3_protocols[protoidx].attr); + else + protostr[0] = '\0'; + virBufferVSprintf(buf, CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR CMD_EXEC "%s" - CMD_DEF("%s -t %s -A %s -p 0x%x -j %s") CMD_SEPARATOR + CMD_DEF("%s -t %s -A %s %s -j %s") CMD_SEPARATOR CMD_EXEC "%s", @@ -2784,7 +2795,7 @@ ebtablesCreateTmpSubChain(virBufferPtr buf, CMD_STOPONERR(stopOnError), ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, - rootchain, l3_protocols[protoidx].attr, chain, + rootchain, protostr, chain, CMD_STOPONERR(stopOnError)); @@ -3357,6 +3368,11 @@ ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED, if (chains_out != 0) ebtablesCreateTmpRootChain(&buf, 0, ifname, 1); + if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_MAC)) + ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_MAC_IDX, 1); + if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_MAC)) + ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_MAC_IDX, 1); + if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4)) ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV4_IDX, 1); if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4)) @@ -3368,10 +3384,14 @@ ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED, ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV6_IDX, 1); /* keep arp,rarp as last */ - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARP_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARP_IDX, 1); + if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_ARPMAC)) + ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARPMAC_IDX, 1); + if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_ARPIP)) + ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARPIP_IDX, 1); + if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_ARPMAC)) + ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARPMAC_IDX, 1); + if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_ARPIP)) + ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARPIP_IDX, 1); if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP)) ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_RARP_IDX, 1); if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP))

13 years, 7 months

1
0
0 / 0

[libvirt] [PATCH 1/9] add DHCP snooping support to nwfilter

by David L Stevens

This patch adds support for "continue" and "return" actions in filter rules. Signed-off-by: David L Stevens <dlstevens(a)us.ibm.com> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 13b5b38..6a15f04 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -54,12 +54,16 @@ VIR_ENUM_IMPL(virNWFilterRuleAction, VIR_NWFILTER_RULE_ACTION_LAST, "drop", "accept", - "reject"); + "reject", + "return", + "continue"); VIR_ENUM_IMPL(virNWFilterJumpTarget, VIR_NWFILTER_RULE_ACTION_LAST, "DROP", "ACCEPT", - "REJECT"); + "REJECT", + "RETURN", + "CONTINUE"); VIR_ENUM_IMPL(virNWFilterRuleDirection, VIR_NWFILTER_RULE_DIRECTION_LAST, "in", diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 40da8c3..ab9d4c1 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -291,6 +291,8 @@ enum virNWFilterRuleActionType { VIR_NWFILTER_RULE_ACTION_DROP = 0, VIR_NWFILTER_RULE_ACTION_ACCEPT, VIR_NWFILTER_RULE_ACTION_REJECT, + VIR_NWFILTER_RULE_ACTION_RETURN, + VIR_NWFILTER_RULE_ACTION_CONTINUE, VIR_NWFILTER_RULE_ACTION_LAST, };

13 years, 7 months

1
0
0 / 0

[libvirt] [PATCH] build: Fix problem of building Python bindings

by Osier Yang

If one specify "--with-python=yes" but no python-devel package is installed, we ignore it with just a notice message, which doesn't give clear guide to user. --- configure.ac | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 190bf40..758c893 100644 --- a/configure.ac +++ b/configure.ac @@ -1917,15 +1917,13 @@ if test "$with_python" != "no" ; then then PYTHON_INCLUDES=-I/usr/include/python$PYTHON_VERSION else - AC_MSG_NOTICE([Could not find python$PYTHON_VERSION/Python.h, disabling bindings]) - with_python=no + AC_MSG_ERROR([You must install python-devel to build Python bindings]) fi fi fi fi else - AC_MSG_NOTICE([Could not find python interpreter, disabling bindings]) - with_python=no + AC_MSG_ERROR([You must install python to build Python bindings]) fi else AC_MSG_NOTICE([Could not find python in $with_python, disabling bindings]) -- 1.7.4

13 years, 7 months

3
4
0 / 0

[libvirt] Magic error introduced by commit f09accc

by Jiri Denemark

Hi all, After commit f09accc (buf: add virBufferVasprintf) libvirt no longer compiles with -Werror because of the following error: CC libvirt_util_la-macvtap.lo cc1: warnings being treated as errors /usr/include/netlink/object.h:58: error: inline function 'nl_object_priv' declared but never defined I must admit I don't understand why including stdarg.h (which seems to be the only change visible in macvtap.c) results in this warning. Does anyone have an idea? Jirka

13 years, 7 months

3
6
0 / 0

[libvirt] libvirt.pc file

by Eric Blake

Why is libvirt.pc included in the tarball? It contains information learned at configure time, thus its contents depend on who built the tarball, which is in violation of the normal rule that any generated file that depends on config.status must not be shipped. Unless anyone has a good reason why it is distributed, I'm working on a patch to fix that. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

13 years, 7 months

4
8
0 / 0

[libvirt] virsh nodedev-dempxml is not supported by the connection driver

by guan qin

Hi, I meet a problem when I use the command "virsh nodedev-dumpxml pci_0000_00_19_0",it shows as follows: root@shahe26:~# virsh nodedev-list error: Failed to count node devices error: this function is not supported by the connection driver: virNodeNumOfDevices root@shahe26:~# virsh nodedev-dumpxml pci_0000_00_19_0 error: Could not find matching device 'pci_0000_00_19_0' error: this function is not supported by the connection driver: virNodeDeviceLookupByName root@shahe26:~# virsh -version 0.9.0 root@shahe26:~# virsh net-list Name State Autostart ----------------------------------------- default active yes root@shahe26:~# virsh list Id Name State ---------------------------------- root@shahe26:~# I find that only the command "virsh nodedev-? ***" can't work . How can I solve this problem ? thanks. best regards, qinguan

13 years, 7 months

2
5
0 / 0

[libvirt] [PATCH] json: Fix *GetBoolean functions

by Jiri Denemark

They were not used anywhere so far so nobody noticed they are broken. --- src/util/json.c | 11 ++++++----- src/util/json.h | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/util/json.c b/src/util/json.c index 0daeae9..df4771d 100644 --- a/src/util/json.c +++ b/src/util/json.c @@ -491,12 +491,13 @@ int virJSONValueGetNumberDouble(virJSONValuePtr number, double *value) } -int virJSONValueGetBoolean(virJSONValuePtr val) +int virJSONValueGetBoolean(virJSONValuePtr val, bool *value) { - if (val->type != VIR_JSON_TYPE_NUMBER) + if (val->type != VIR_JSON_TYPE_BOOLEAN) return -1; - return val->data.boolean; + *value = val->data.boolean; + return 0; } @@ -593,7 +594,7 @@ int virJSONValueObjectGetNumberDouble(virJSONValuePtr object, const char *key, d } -int virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key) +int virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key, bool *value) { virJSONValuePtr val; if (object->type != VIR_JSON_TYPE_OBJECT) @@ -603,7 +604,7 @@ int virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key) if (!val) return -1; - return virJSONValueGetBoolean(val); + return virJSONValueGetBoolean(val, value); } diff --git a/src/util/json.h b/src/util/json.h index ea28de6..4572654 100644 --- a/src/util/json.h +++ b/src/util/json.h @@ -105,7 +105,7 @@ int virJSONValueGetNumberUint(virJSONValuePtr object, unsigned int *value); int virJSONValueGetNumberLong(virJSONValuePtr object, long long *value); int virJSONValueGetNumberUlong(virJSONValuePtr object, unsigned long long *value); int virJSONValueGetNumberDouble(virJSONValuePtr object, double *value); -int virJSONValueGetBoolean(virJSONValuePtr object); +int virJSONValueGetBoolean(virJSONValuePtr object, bool *value); int virJSONValueIsNull(virJSONValuePtr object); const char *virJSONValueObjectGetString(virJSONValuePtr object, const char *key); @@ -114,7 +114,7 @@ int virJSONValueObjectGetNumberUint(virJSONValuePtr object, const char *key, uns int virJSONValueObjectGetNumberLong(virJSONValuePtr object, const char *key, long long *value); int virJSONValueObjectGetNumberUlong(virJSONValuePtr object, const char *key, unsigned long long *value); int virJSONValueObjectGetNumberDouble(virJSONValuePtr object, const char *key, double *value); -int virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key); +int virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key, bool *value); int virJSONValueObjectIsNull(virJSONValuePtr object, const char *key); int virJSONValueObjectAppendString(virJSONValuePtr object, const char *key, const char *value); -- 1.7.5.rc3

13 years, 7 months

2
2
0 / 0

[libvirt] How to suppress error printing to stderr from virConnectOpen*?

by Richard W.M. Jones

I can set the global error handling function (virSetErrorFunc). That doesn't seem to be a good idea from a library. Looking at the code, it doesn't appear that the internal virErrorHandler is thread-local, so I can't set it and restore it around the function call. I can set the error handling function _after_ I've got a connection object (virConnSetErrorFunc). That doesn't help with making the initial connection. libguestfs itself has the same problem with the guestfs_create call, so I'm not going to point any fingers :-) However it seems like a shortcoming of the libvirt API. Am I missing something here? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw

13 years, 7 months

2
2
0 / 0

[libvirt] [PATCH] Allow destroying QEMU VM even if a job is active

by Daniel P. Berrange

Introduce a virProcessKill function that can be safely called even when the job mutex is held. This allows virDomainDestroy to kill any VM even if it is asleep in a monitor job. The PID will die and the thread asleep on the monitor will then wake up releasing the job mutex. * src/qemu/qemu_driver.c: Kill process before using qemuProcessStop to ensure job is released * src/qemu/qemu_process.c: Add virProcessKill for killing off QEMU processes --- src/qemu/qemu_driver.c | 7 +++++++ src/qemu/qemu_process.c | 39 +++++++++++++++++++++++++++++++-------- src/qemu/qemu_process.h | 2 ++ 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3f9e00..6d6fb51 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1482,6 +1482,13 @@ static int qemudDomainDestroy(virDomainPtr dom) { goto cleanup; } + /* Although qemuProcessStop does this already, there may + * be an outstanding job active. We want to make sure we + * can kill the process even if a job is active. Killing + * it now, means the job will be released + */ + qemuProcessKill(vm); + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a6c0dc8..c60c51f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2369,6 +2369,36 @@ cleanup: } +void qemuProcessKill(virDomainObjPtr vm) +{ + int i; + int rc; + VIR_DEBUG("vm=%s pid=%d", vm->def->name, vm->pid); + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("VM '%s' not active", vm->def->name); + return; + } + + for (i = 0 ; i < 15 ; i++) { + int signum; + if (i == 0) + signum = SIGTERM; + else if (i == 8) + signum = SIGKILL; + else + signum = 0; /* Just check for existance */ + + rc = virKillProcess(vm->pid, signum); + VIR_DEBUG("Iteration %d rc=%d", i, rc); + if (rc < 0) + break; + + usleep(200 * 1000); + } +} + + void qemuProcessStop(struct qemud_driver *driver, virDomainObjPtr vm, int migrated) @@ -2436,13 +2466,6 @@ void qemuProcessStop(struct qemud_driver *driver, } } - /* This will safely handle a non-running guest with pid=0 or pid=-1*/ - if (virKillProcess(vm->pid, 0) == 0 && - virKillProcess(vm->pid, SIGTERM) < 0) - virReportSystemError(errno, - _("Failed to send SIGTERM to %s (%d)"), - vm->def->name, vm->pid); - if (priv->mon) qemuMonitorClose(priv->mon); @@ -2454,7 +2477,7 @@ void qemuProcessStop(struct qemud_driver *driver, } /* shut it off for sure */ - virKillProcess(vm->pid, SIGKILL); + qemuProcessKill(vm); /* now that we know it's stopped call the hook if present */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index f1ab599..d8afab0 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -49,4 +49,6 @@ void qemuProcessStop(struct qemud_driver *driver, virDomainObjPtr vm, int migrated); +void qemuProcessKill(virDomainObjPtr vm); + #endif /* __QEMU_PROCESS_H__ */ -- 1.7.4.4

13 years, 7 months

2
2
0 / 0

[libvirt] RFC: APIs for managing a subset of a domain's disks

by Eric Blake

Consider the case of a guest that has multiple virtual disks, some residing on shared storage (such as the OS proper) and some on local storage (scratch space, where the OS has faster response if the virtual disk does not have to go over the network, and possibly one where the guest can still work even if the disk is hot-unplugged). During migration, you'd want different handling of the two disks (the destination can already see the shared disk, but must either copy the contents or recreate a blank scratch volume for the local disk). Or, consider the case where a guest has one disk as qcow2 (it is not modified frequently, and benefits from sharing a common backing file with other guests), while another disk is raw (for better read-write performance). Right now, 'virsh snapshot' fails, because it only works if all disks are qcow2; and in fact it may be the case that it is desirable to only take a snapshot of a subset of the domain's disks. So, I think we need some way to request an operation on a subset of VM disks, in a manner that can be shared between migration and volume management APIs. And I'm not sure it makes sense to add two more parameters to migration commands (an array of disks, and the size of that array), nor to modify the snapshot XML to describe which disks belong to the snapshot. So I'm thinking we need some sort of API set to manage a stateful set of disk operations. Maybe the trick is to define that every VM has a (possibly empty) set of selected disks, with APIs to manage moving a single disk in or out of the set, an API for listing the entire set, then a single flag to migration that states that live block migration is attempted for all disks currently in the VMs selected disk set. Being stateful, this would have to be represented in XML (so that if libvirtd is restarted, it remembers which disks are selected); I'm thinking of adding a new selected='yes|no' attribute to <disk>, as in: <disk type='file' device='disk' selected='yes'/> <driver name='qemu' type='raw'/> ... </disk> where if the attribute is absent, it defaults to no. For hypervisors where the state is maintained by libvirtd (qemu, lxc), the XML works; for other hypervisors, the notion of a subset of selected disks would have to just fail unless there is some hypervisor-specific way to track that information alongside a domain. For my API proposal, I'm including an unused flags argument to all the virDomainDiskSet* commands (experience has taught me well). In fact, we could even use that flags parameter, to maintain parallel sets (set 0 is the set of disks to migrate, set 1 is the set of disks to snapshot, ...), although I don't think we need that complexity yet (besides, it would affect the proposed XML). /* Add disk to the domain's set of selected disks; flags ignored for now; return 0 on success, 1 if already in the set, -1 on failure */ int virDomainDiskSetAdd(virDomainPtr dom, char *disk, unsigned int flags); /* Remove disk from the domain's set of selected disks; flags ignored for now; return 0 on success, 1 if already absent from the set, -1 on failure */ int virDomainDiskSetRemove(virDomainPtr dom, char *disk, unsigned int flags); /* Add all disks to the domain's set of selected disks; flags ignored for now; return 0 on success, -1 on failure */ int virDomainDiskSetAddAll(virDomainPtr dom, unsigned int flags); /* Remove all disks from the domain's set of selected disks; flags ignored for now; return 0 on success, -1 on failure */ int virDomainDiskSetRemoveAll(virDomainPtr dom, unsigned int flags); /* Return the size of the domain's currently selected disk set, or -1 on failure; flags ignored for now */ int virDomainDiskSetSize(virDomainPtr dom, unsigned int flags); /* Populate up to n entries of the array with the names of the domain's selected disk set, and return how many entries were populated, or -1 on failure; flags ignored for now */ int virDomainDiskSetList(virDomainPtr dom, char **array, int n, unsigned int flags) With API in place for tracking a subset of selected disks, we can then extend existing APIs with new flags: /* Old way - domain migration without any disks migrated */ virDomainMigrate(dom, dconn, flags | 0, dname, uri, bandwidth) /* New way - domain migration, including all disks in the domain's selected disk set being copied to the destination */ virDomainMigrate(dom, dconn, flags | VIR_MIGRATE_WITH_DISK_SET, dname, uri, bandwidth) /* Old way - snapshot of all disks */ virDomainSnapshotCreateXML(dom, xml, 0) /* New way - snapshot of just disks in selected disk set */ virDomainSnapshotCreateXML(dom, xml, VIR_DOMAIN_SAVE_DISK_SET) I'd also like to see some collaboration between virDomainSave (for memory) and virDomainSmapshotCreateXML (for disks); unfortunately, virDomainSave doesn't take a flags argument. Maybe this calls for a new API, and possibly a new version of the header to a 'virsh save' image to track the location of snapshotted disks alongside the saved memory state: /* Save the RAM state of domain to the base file "to". If "xml" is NULL, no disks are snapshotted. Otherwise, "xml" is a snapshot XML that describes how disk state will also be saved; if flags includes VIR_DOMAIN_SAVE_DISK_SET, then the domain's selected disk set is snapshotted, otherwise all disks are snapshotted. If flags contains VIR_DOMAIN_SAVE_LIVE, then the guest is resumed after snapshot is completed; otherwise the guest is halted. */ int virDomainSaveFlags(virDomainPtr dom, const char *to, const char *xml, unsigned int flags); Thoughts before I start implementing some of this for post-0.9.1? -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

13 years, 7 months

4
5
0 / 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Devel May 2011