- Devel - libvirt List Archives

[libvirt] [PATCH v2 0/2] Revert "configure: Remove --enable-test-coverage"
by Jiri Denemark 13 Aug '19

13 Aug '19

Gnulib's make coverage (or init-coverage, build-coverage, gen-coverage) is not a 1-1 replacement for the original configure option. Our old --enable-test-coverage seems to be close to gnulib's make build-coverage except gnulib runs lcov in that phase and the build actually fails for me even before lcov is run. And since we want to be able to just build libvirt without running lcov, I suggest reverting to our own implementation. I admit the best solution would be to somehow make gnulib support what we need (and fix the actual build), but I don't feel brave enough to do that. Eric? :-) Also if we ever switch to glib and drop gnulib completely, we would need to reintroduce our own implementation anyway. I had to do some changes to resolve conflicts. The changes were not too big, but I figured I'd rather resend this patch especially when I originally forgot to Cc the original author of the reverted patch. Jiri Denemark (2): Revert "configure: Remove --enable-test-coverage" Revert "docs: hacking: Add 'Code coverage reports' section" Makefile.am | 20 +++++++++++++++++--- configure.ac | 18 ++++++++++++++++++ docs/hacking.html.in | 29 ----------------------------- src/Makefile.am | 2 ++ src/remote/Makefile.inc.am | 2 ++ tests/Makefile.am | 2 ++ tools/Makefile.am | 6 ++++++ 7 files changed, 47 insertions(+), 32 deletions(-) -- 2.22.1

2 3

[libvirt] [PATCH] test_driver: implement virDomainSetLifecycleAction
by Ilias Stamatis 13 Aug '19

13 Aug '19

Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com> --- src/test/test_driver.c | 58 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index aae9875194..71d6baa3da 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7404,6 +7404,63 @@ testDomainMemoryPeek(virDomainPtr dom, } +static void +testDomainModifyLifecycleAction(virDomainDefPtr def, + virDomainLifecycle type, + virDomainLifecycleAction action) +{ + switch (type) { + case VIR_DOMAIN_LIFECYCLE_POWEROFF: + def->onPoweroff = action; + break; + case VIR_DOMAIN_LIFECYCLE_REBOOT: + def->onReboot = action; + break; + case VIR_DOMAIN_LIFECYCLE_CRASH: + def->onCrash = action; + break; + case VIR_DOMAIN_LIFECYCLE_LAST: + break; + } +} + + +static int +testDomainSetLifecycleAction(virDomainPtr dom, + unsigned int type, + unsigned int action, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + virDomainDefPtr def = NULL; + virDomainDefPtr persistentDef = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!virDomainDefLifecycleActionAllowed(type, action)) + return -1; + + if (!(vm = testDomObjFromDomain(dom))) + return -1; + + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) + goto cleanup; + + if (def) + testDomainModifyLifecycleAction(def, type, action); + + if (persistentDef) + testDomainModifyLifecycleAction(persistentDef, type, action); + + ret = 0; + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + /* * Snapshot APIs */ @@ -8698,6 +8755,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.4 */ .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.4 */ .domainMemoryPeek = testDomainMemoryPeek, /* 5.4.0 */ + .domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */ .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */ .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */ -- 2.22.0

2 3

[libvirt] [PATCH 0/2] test_driver: implement Get/Set Blkio parameters APIs
by Ilias Stamatis 13 Aug '19

13 Aug '19

This series depends on another series that haven't been merged yet: https://www.redhat.com/archives/libvir-list/2019-July/msg00520.html Ilias Stamatis (2): test_driver: implement virDomainGetBlkioParameters test_driver: implement virDomainSetBlkioParameters src/test/test_driver.c | 123 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) -- 2.22.0

1 2

[libvirt] [PATCH v2]daemon: Fix a crash during virNetlinkEventServiceStopAll
by Liu Haitao 12 Aug '19

12 Aug '19

When reboot the host, a core dump file would be generated. The call traces are: Note.In this case, the main thread is thread 5. (gdb) thread 5 [Switching to thread 5 (LWP 4142)] (gdb) bt 0 0x00007f00a6838273 in futex_wait_cancelable (private=<optimized out>, expected=0, futex_word=0x7f004c0125c0) at /usr/src/debug/glibc/2.24-r0/git/sysdeps/unix/sysv/linux/futex-internal.h:88 1 __pthread_cond_wait_common (abstime=0x0, mutex=0x7f004c012540, cond=0x7f004c012598) at /usr/src/debug/glibc/2.24-r0/git/nptl/pthread_cond_wait.c:502 2 __pthread_cond_wait (cond=0x7f004c012598, mutex=0x7f004c012540) at /usr/src/debug/glibc/2.24-r0/git/nptl/pthread_cond_wait.c:655 3 0x00007f00aa467246 in virCondWait (c=<optimized out>, m=<optimized out>) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virthread.c:154 4 0x00007f00aa467eb0 in virThreadPoolFree (pool=<optimized out>) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virthreadpool.c:286 5 0x00007f0074349f9d in qemuStateCleanup () at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/qemu/qemu_driver.c:1036 6 0x00007f00aa5e9486 in virStateCleanup () at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/libvirt.c:682 7 0x000055a687ab86a4 in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/remote/remote_daemon.c:1473 (gdb) thread 1 [Switching to thread 1 (LWP 4403)] (gdb) bt 0 __GI___pthread_mutex_lock (mutex=mutex@entry=0x0) at /usr/src/debug/glibc/2.24-r0/git/nptl/pthread_mutex_lock.c:67 1 0x00007f00aa467165 in virMutexLock (m=m@entry=0x0) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virthread.c:89 2 0x00007f00aa43c0f9 in virNetlinkEventServerLock (driver=<optimized out>) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virnetlink.c:799 3 virNetlinkEventRemoveClient (watch=watch@entry=0, macaddr=macaddr@entry=0x7f0088014944, protocol=protocol@entry=0) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virnetlink.c:1197 4 0x00007f00aa4341df in virNetDevMacVLanDeleteWithVPortProfile ( ifname=<optimized out>, macaddr=macaddr@entry=0x7f0088014944, linkdev=0x7f0088014920 "eth1", mode=mode@entry=1, virtPortProfile=virtPortProfile@entry=0x0, stateDir=stateDir@entry=0x7f004c12fa90 "/var/run/libvirt/qemu") at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virnetdevmacvlan.c:1112 5 0x00007f0074312251 in qemuProcessStop (driver=driver@entry=0x7f004c0ecef0, vm=vm@entry=0x7f0088000b00, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN, asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=<optimized out>) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/qemu/qemu_process.c:7291 6 0x00007f007437a5ea in processMonitorEOFEvent (vm=0x7f0088000b00, driver=0x7f004c0ecef0) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/qemu/qemu_driver.c:4756 7 qemuProcessEventHandler (data=0x55a687d6df10, opaque=0x7f004c0ecef0) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/qemu/qemu_driver.c:4859 8 0x00007f00aa467c5b in virThreadPoolWorker ( opaque=opaque@entry=0x55a687d6c110) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virthreadpool.c:163 9 0x00007f00aa466fe8 in virThreadHelper (data=<optimized out>) at /usr/src/debug/libvirt/5.3.0-r0/libvirt-5.3.0/src/util/virthread.c:206 10 0x00007f00a68323f4 in start_thread (arg=0x7f00699df700) at /usr/src/debug/glibc/2.24-r0/git/nptl/pthread_create.c:456 11 0x00007f00a616e10f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:105 1. The execution flow of main thread (Thread 5 LWP 4142): main() -->virNetDaemonRun() -->virNetDaemonClose(dmn) //cleanup -->virNetlinkEventServiceStopAll() -->virStateCleanup() -->qemuStateCleanup() -->virThreadPoolFree() -->__pthread_cond_wait() virNetDaemonRun() -->virEventRunDefaultImpl -->virEventPollRunOnce -->virEventPollDispatchHandles -->qemuMonitorIO -->qemuProcessHandleMonitorEOF -->processEvent->eventType = QEMU_PROCESS_EVENT_MONITOR_EOF -->virThreadPoolSendJob() After typing reboot command on the host, the main thread would send an event message to another thread. Here it would let thread 1 to handle the shutdown of qemu process. But it could not be executed immediately. virNetlinkEventServiceStopAll() --> virNetlinkEventServiceStop() --> server[protocol] = NULL; // set server to null IN virNetlinkEventServiceStopAll(), some variables related to network are freed, like (static virNetlinkEventSrvPrivatePtr server). virStateCleanup() -->qemuStateCleanup() -->virThreadPoolFree() -->__pthread_cond_wait() In virThreadPoolFree() it will wait other thread to end up. 2. The execution flow of thread 5 (LWP 4403): qemuProcessStop() -->virNetDevMacVLanDeleteWithVPortProfile() -->virNetlinkEventRemoveClient() --> srv = server[protocol] Although the main thread had sent the message to thread 1(4403), it could not be run instantly. It means that the virNetlinkEventServiceStopAll() might be executed earlier than virNetlinkEventRemoveClient(). We could get it from the log file. "" 2019-06-12 00:10:09.230+0000: 4142: info : virNetlinkEventServiceStopAll:941 : stopping all netlink event services 2019-06-12 00:10:09.230+0000: 4142: info : virNetlinkEventServiceStop:904 : stopping netlink event service 2019-06-12 00:10:21.165+0000: 4403: debug : virNetlinkEventRemoveClient:1190 : removing client watch=0, mac=0x7f0088014944. " In virNetlinkEventRemoveClient() the variable server is used again, but now it is null that is freed by virNetlinkEventServiceStopAll().So it would case a crash . The virNetlinkEventServiceStopAll() should be executed behind virStateCleanup(), Signed-off-by: Liu Haitao <haitao.liu(a)windriver.com> --- src/remote/remote_daemon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index c3782971f1..7da20a6644 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -1464,8 +1464,6 @@ int main(int argc, char **argv) { /* Keep cleanup order in inverse order of startup */ virNetDaemonClose(dmn); - virNetlinkEventServiceStopAll(); - if (driversInitialized) { /* NB: Possible issue with timing window between driversInitialized * setting if virNetlinkEventServerStart fails */ @@ -1473,6 +1471,8 @@ int main(int argc, char **argv) { virStateCleanup(); } + virNetlinkEventServiceStopAll(); + virObjectUnref(adminProgram); virObjectUnref(srvAdm); virObjectUnref(qemuProgram); -- 2.21.0

2 3

[libvirt] [PATCH] Revert "tpm: Check TPM XML device configuration changes after edit"
by Ján Tomko 12 Aug '19

12 Aug '19

Redefining a domain via virDomainDefineXML should not give different results based on an already existing definition. Also, there's a crasher somewhere in the code: https://bugzilla.redhat.com/show_bug.cgi?id=1739338 This reverts commit 94b3aa55f83ada33a9fdda66068d58ef1a56c0a5 --- src/conf/domain_conf.c | 56 --------------------------------------- src/conf/domain_conf.h | 3 --- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 28 -------------------- src/qemu/qemu_extdevice.c | 2 +- src/qemu/qemu_extdevice.h | 3 --- 6 files changed, 1 insertion(+), 92 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0456369d55..f54acd0c2a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31461,59 +31461,3 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics) return true; } - - -static int -virDomainCheckTPMChanges(virDomainDefPtr def, - virDomainDefPtr newDef) -{ - bool oldEnc, newEnc; - - if (!def->tpm) - return 0; - - switch (def->tpm->type) { - case VIR_DOMAIN_TPM_TYPE_EMULATOR: - if (virFileExists(def->tpm->data.emulator.storagepath)) { - /* VM has been started */ - /* Once a VM was started with an encrypted state we allow - * less configuration changes. - */ - oldEnc = def->tpm->data.emulator.hassecretuuid; - if (oldEnc && def->tpm->type != newDef->tpm->type) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Changing the type of TPM is not allowed")); - return -1; - } - if (oldEnc && !newDef->tpm) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Removing an encrypted TPM is not allowed")); - return -1; - } - newEnc = newDef->tpm->data.emulator.hassecretuuid; - if (oldEnc != newEnc) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("TPM state encryption cannot be changed " - "once VM was started")); - return -1; - } - } - break; - case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - case VIR_DOMAIN_TPM_TYPE_LAST: - break; - } - - return 0; -} - - -int -virDomainCheckDeviceChanges(virDomainDefPtr def, - virDomainDefPtr newDef) -{ - if (!def || !newDef) - return 0; - - return virDomainCheckTPMChanges(def, newDef); -} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 57ca2a8ad1..2d7350e675 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3641,6 +3641,3 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics); bool virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics); - -int -virDomainCheckDeviceChanges(virDomainDefPtr def, virDomainDefPtr newDef); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cae8febf8d..7a3feb8efa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -217,7 +217,6 @@ virDomainBootTypeFromString; virDomainBootTypeToString; virDomainCapabilitiesPolicyTypeToString; virDomainCapsFeatureTypeToString; -virDomainCheckDeviceChanges; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ff83d1c024..0fccf3dc37 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -53,7 +53,6 @@ #include "qemu_migration_params.h" #include "qemu_blockjob.h" #include "qemu_security.h" -#include "qemu_extdevice.h" #include "virerror.h" #include "virlog.h" @@ -7760,30 +7759,6 @@ qemuDomainCreate(virDomainPtr dom) return qemuDomainCreateWithFlags(dom, 0); } -static int -qemuDomainCheckDeviceChanges(virQEMUDriverPtr driver, - virDomainDefPtr def) -{ - virDomainObjPtr vm; - int ret; - - vm = virDomainObjListFindByUUID(driver->domains, def->uuid); - if (!vm) - return 0; - - if (qemuExtDevicesInitPaths(driver, vm->def) < 0) { - ret = -1; - goto cleanup; - } - - ret = virDomainCheckDeviceChanges(vm->def, def); - - cleanup: - virDomainObjEndAPI(&vm); - - return ret; -} - static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml, @@ -7820,9 +7795,6 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup; - if (qemuDomainCheckDeviceChanges(driver, def) < 0) - goto cleanup; - if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, 0, &oldDef))) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index af52466421..dc032aa60c 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -79,7 +79,7 @@ qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, * stored and we can remove directories and files in case of domain XML * changes. */ -int +static int qemuExtDevicesInitPaths(virQEMUDriverPtr driver, virDomainDefPtr def) { diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 5a53c79f38..039b3e60dd 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -54,6 +54,3 @@ bool qemuExtDevicesHasDevice(virDomainDefPtr def); int qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver, virDomainDefPtr def, virCgroupPtr cgroup); - -int qemuExtDevicesInitPaths(virQEMUDriverPtr driver, - virDomainDefPtr def); -- 2.19.2

5 6

[libvirt] [PATCH v2 0/5] test_driver: implement IOThread related APIs
by Ilias Stamatis 12 Aug '19

12 Aug '19

Changes since v1: - Use vm-private data for storing iothread parameters and implement virDomainSetIOThreadParams Ilias Stamatis (5): test_driver: implement virDomainAddIOThread test_driver: implement virDomainDelIOThread test_driver: implement virDomainPinIOThread test_driver: implement virDomainGetIOThreadInfo test_driver: implement virDomainSetIOThreadParams src/test/test_driver.c | 362 +++++++++++++++++++++++++++++++++++++++++ src/test/test_driver.h | 11 ++ 2 files changed, 373 insertions(+) -- 2.22.0

1 5

[libvirt] [PATCH v2 0/2] add support for kvm-hint-dedicated performance hint
by Menno Lageman 12 Aug '19

12 Aug '19

v2 of: https://www.redhat.com/archives/libvir-list/2019-August/msg00363.html diff to v1: - reword description in documentation - move check for valid domain config to qemuDomainDefValidate Wim ten Have (2): qemu: support for kvm-hint-dedicated performance hint tests: add tests for kvm-hint-dedicated feature docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ src/qemu/qemu_domain.c | 11 ++++++++++- tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 4 ++-- tests/qemuxml2argvdata/kvm-features.xml | 4 +++- tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 4 +++- 11 files changed, 42 insertions(+), 5 deletions(-) -- 2.21.0

2 3

[libvirt] [PATCH 0/2] add support for kvm-hint-dedicated performance hint
by Menno Lageman 12 Aug '19

12 Aug '19

QEMU version 2.12.1 introduced a performance feature under commit be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint") This patch adds a new KVM feature 'hint-dedicated' to set this performance hint for KVM guests. Wim ten Have (2): qemu: support for kvm-hint-dedicated performance hint tests: add tests for kvm-hint-dedicated feature docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 4 ++-- tests/qemuxml2argvdata/kvm-features.xml | 4 +++- tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 4 +++- 10 files changed, 40 insertions(+), 4 deletions(-) -- 2.21.0

2 7

[libvirt] [PATCH v2 0/2] test_driver: implement get/set BlockIoTune APIs
by Ilias Stamatis 12 Aug '19

12 Aug '19

Ilias Stamatis (2): test_driver: implement testDomainGetBlockIoTune test_driver: implement virDomainSetBlockIoTune src/test/test_driver.c | 361 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 361 insertions(+) -- 2.22.0

2 5

[libvirt] [PATCH] util: allow tap-based guest interfaces to have MAC address prefix 0xFE
by Laine Stump 12 Aug '19

12 Aug '19

Back in July 2010, commit 6ea90b84 (meant to resolve https://bugzilla.redhat.com/571991 ) added code to set the MAC address of any tap device to the associated guest interface's MAC, but with the first byte replaced with 0xFE. This was done in order to assure that 1) the tap MAC and guest interface MAC were different (otherwise L2 forwarding through the tap would not work, and the kernel would repeatedly issue a warning stating as much). 2) any bridge device that had one of these taps attached would *not* take on the MAC of the tap (leading to network instability as guests started and stopped) A couple years later, https://bugzilla.redhat.com/798467 was filed, complaining that a user could configure a tap-based guest interface to have a MAC address that itself had a first byte of 0xFE, silently (other than the kernel warning messages) resulting in a non-working configuration. This was fixed by commit 5d571045, which logged an error and failed the guest start / interface attach if the MAC's first byte was 0xFE. Although this restriction only reduces the potential pool of MAC addresses from 2^46 (last two bits of byte 1 must be set to 10) by 2^32 (still 4 orders of magnitude larger than the entire IPv4 address space), it also means that management software that autogenerates MAC addresses must have special code to avoid an 0xFE prefix. Now after 7 years, someone has noticed this restriction and requested that we remove it. So instead of failing when 0xFE is found as the first byte, this patch removes the restriction by just replacing the first byte in the tap device MAC with 0xFA if the first byte in the guest interface is 0xFE. 0xFA is the next-highest value that still has 10 as the lowest two bits, and still 2) meets the requirement of "tap MAC must be different from guest interface MAC", and 3) is high enough that there should never be an issue of the attached bridge device taking on the MAC of the tap. The result is that *any* MAC can be chosen by management software (although it would still not work correctly if a multicast MAC (lowest bit of first byte set to 1) was chosen), but that's a different issue). Signed-off-by: Laine Stump <laine(a)redhat.com> --- Yes, I find it slightly problematic that the same setting is in two different places in the code, but 1) that is pre-existing, and 2) if I moved the MAC address setting down one level into virNetDevTapCreate(), the code would *still* need to be duplicated, since there are two different implementations of virNetDevTapCreate(). If anyone is bothered by this, then I can resubmit with an extra patch to add a new virNetDevTapCreate() that just calls a platform-specific virNetDevTapCreateInternal(), then sets the tap mac address. Only by request though :-). src/qemu/qemu_interface.c | 11 ++++++++++- src/util/virnetdevtap.c | 26 ++++++++++++-------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index c8effa68f4..72ed51cb1f 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -444,8 +444,17 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, } virDomainAuditNetDevice(def, net, tunpath, true); + + /* The tap device's MAC address cannot match the MAC address + * used by the guest. This results in "received packet on + * vnetX with own address as source address" error logs from + * the kernel. + */ virMacAddrSet(&tapmac, &net->mac); - tapmac.addr[0] = 0xFE; + if (tapmac.addr[0] == 0xFE) + tapmac.addr[0] = 0xFA; + else + tapmac.addr[0] = 0xFE; if (virNetDevSetMAC(net->ifname, &tapmac) < 0) goto cleanup; diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index b65c26bee1..4548b51b5b 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -668,7 +668,6 @@ int virNetDevTapCreateInBridgePort(const char *brname, unsigned int flags) { virMacAddr tapmac; - char macaddrstr[VIR_MAC_STRING_BUFLEN]; size_t i; if (virNetDevTapCreate(ifname, tunpath, tapfd, tapfdSize, flags) < 0) @@ -682,19 +681,18 @@ int virNetDevTapCreateInBridgePort(const char *brname, */ virMacAddrSet(&tapmac, macaddr); if (!(flags & VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE)) { - if (macaddr->addr[0] == 0xFE) { - /* For normal use, the tap device's MAC address cannot - * match the MAC address used by the guest. This results - * in "received packet on vnetX with own address as source - * address" error logs from the kernel. - */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unable to use MAC address starting with " - "reserved value 0xFE - '%s' - "), - virMacAddrFormat(macaddr, macaddrstr)); - goto error; - } - tapmac.addr[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */ + /* The tap device's MAC address cannot match the MAC address + * used by the guest. This results in "received packet on + * vnetX with own address as source address" error logs from + * the kernel. Making the tap address as high as possible + * discourages the bridge from using this tap's MAC as its own + * (a Linux host bridge will take on the lowest numbered MAC + * of all devices attached to it). + */ + if (tapmac.addr[0] == 0xFE) + tapmac.addr[0] = 0xFA; + else + tapmac.addr[0] = 0xFE; } if (virNetDevSetMAC(*ifname, &tapmac) < 0) -- 2.21.0

2 1