Re: [libvirt] [libvirt-python][PATCH] Implement virDomain{Get, Set}Time APIs
On 19.5.2014 16:07, Michal Privoznik wrote: > While the setter can be generated automatically, the getter is not. > However, it would be a lot easier if they both share the same logic: > a python dictionary to represent the time: dict['seconds'] to > represent seconds, and dict['nseconds'] to represent microseconds. > > Signed-off-by: Michal Privoznik > --- > generator.py | 2 + > libvirt-override-virDomain.py | 13 ++ > libvirt-override.c| 95 > +++ > sanitytest.py | 2 +- > 4 files changed, 111 insertions(+), 1 deletion(-) > ACK Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5] migration: add support for migrateURI configuration
s/migrateURI/migrateHost/ in Subject. On Tue, May 20, 2014 at 14:08:05 +0800, Chen Fan wrote: > For now, we set the migration URI via command line '--migrate_uri' or > construct the URI by looking up the dest host's hostname which could be > solved by DNS automatically. > > But in cases the dest host have two or more NICs to reach, we may need to > send the migration data over a specific NIC which is different from the > automatically resloved one for some reason like performance, security, etc. > thus we must explicitly specify the migrateuri in command line everytime, > but it is too troublesome if there are many such hosts(and don't forget > virt-manager). > > This patch adds a configuration file option on dest host to save the > default value set which can be specified to a migration hostname or > one of this host's addresses used for transferring data, thus user doesn't > boring to specify it in command line everytime. > > Signed-off-by: Chen Fan > --- > > v4-v5: using "migrate_host" instead of "migrate_uri" configuration. Yeah, that's the best solution which I wanted to suggest in a discussion on v4 but I failed to actually send that email. Fortunately, Daniel suggested the same :-) Although I'd call it "migration_host" to be consistent with existing migration_address, migration_port_min, and migration_port_max. > > src/qemu/qemu.conf| 7 ++- > src/qemu/qemu_conf.c | 1 + > src/qemu/qemu_conf.h | 1 + > src/qemu/qemu_migration.c | 25 + > 4 files changed, 29 insertions(+), 5 deletions(-) src/qemu/libvirtd_qemu.aug and src/qemu/test_libvirtd_qemu.aug.in also need to be updated with the new configuration option. > > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf > index f0e802f..421efc4 100644 > --- a/src/qemu/qemu.conf > +++ b/src/qemu/qemu.conf > @@ -449,7 +449,12 @@ > # > #seccomp_sandbox = 1 > > - > +# Override the migration hostname for transfering the migration data. By > +# default, the migrate hostname is set to the host's configured hostname. > +# This can be used to override the default value set by a migration > +# hostname or an IP address of the host machine. both IPv4 and IPv6 > +# addresses are accepted. > +#migrate_host = "localhost" How about: # The default hostname or IP address which will be used by a migration # source for transferring migration data to this host. The migration # source has to be able to resolve this hostname and connect to it so # setting "localhost" will not work. By default, the host's configured # hostname is used. #migration_host = "host.example.com" > > # Override the listen address for all incoming migrations. Defaults to > # 0.0.0.0, or :: if both host and qemu are capable of IPv6. > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index 198ee2f..391fc57 100644 > --- a/src/qemu/qemu_conf.c > +++ b/src/qemu/qemu_conf.c > @@ -574,6 +574,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr > cfg, > > GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox); > > +GET_VALUE_STR("migrate_host", cfg->migrateHost); s/migrate_host/migration_host/ > GET_VALUE_STR("migration_address", cfg->migrationAddress); > > ret = 0; > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h > index a36ea63..8e872b9 100644 > --- a/src/qemu/qemu_conf.h > +++ b/src/qemu/qemu_conf.h > @@ -163,6 +163,7 @@ struct _virQEMUDriverConfig { > > int seccompSandbox; > > +char *migrateHost; > /* The default for -incoming */ > char *migrationAddress; > int migrationPortMin; > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index a9f7fea..963d1ef 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -2639,6 +2639,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, > int ret = -1; > virURIPtr uri = NULL; > bool well_formed_uri = true; > +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > +const char *migrateHost = cfg->migrateHost; > > VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " >"cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " > @@ -2652,8 +2654,9 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, > /* The URI passed in may be NULL or a string "tcp://somehostname:port". > * > * If the URI passed in is NULL then we allocate a port number > - * from our pool of port numbers and return a URI of > - * "tcp://ourhostname:port". > + * from our pool of port numbers, and if the migrateHost is configured, > + * we return a URI of "tcp://migrateHost:port", otherwise return a URI > + * of "tcp://ourhostname:port". > * > * If the URI passed in is not NULL then we try to parse out the > * port number and use that (note that the hostname is assumed > @@ -2663,8 +2666,21 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, > if (virPortAllocatorAcquire(driver
Re: [libvirt] [PATCH] util: refactor virNetlinkCommand to fix several bugs / style problems
On 05/16/2014 06:24 PM, Ján Tomko wrote: > On 05/13/2014 01:51 PM, Laine Stump wrote: >> Inspired by a simpler patch from "Wangrui (K) ". >> >> A submitted patch pointed out that virNetlinkCommand() was doing an >> improper typecast of the return value from nl_recv() (int to >> unsigned), causing it to miss error returns, and that even after >> remedying that problem, virNetlinkCommand() was calling VIR_FREE() on >> the pointer returned from nl_recv() (*resp) even if nl_recv() had >> returned an error, and that in this case the pointer was verifiably >> invalid, as it was pointing to memory that had been allocated by >> libnl, but then freed prior to returning the error. >> >> While reviewing this patch, I noticed several other problems with this >> seemingly simple function (at least one of them as serious as the >> problem being reported/fixed by the aforementioned patch), and decided >> they all deserved to be fixed. Here is the list: >> >> 1) The return value from nl_recv() must be assigned to an int (rather >>than unsigned int) in order to detect failure. >> >> 2) When nl_recv() returns an error, the contents of *resp is invalid, >>and should be simply set to 0, *not* VIR_FREE()'d. >> >> 3) The first error return from virNetlinkCommand returns -EINVAL, >>incorrectly implying that the caller can expect the return value to >>be of the "-errno" variety, which is not true in any other case. >> >> 4) The 2nd error return returns directly with garbage in *resp. While >>the caller should never use *resp in this case, it's still good >>practice to set it to NULL. >> >> 5) For the next 5 (!!) error conditions, *resp will contain garbage, >>and virNetlinkCommand() will goto it's cleanup code which will >>VIR_FREE(*resp), almost surely leading to a segfault. >> >> In addition to fixing these 5 problems, this patch also makes the >> following two changes to make the function conform more closely to the >> style of other libvirt code: >> >> 1) Change the handling of return code from "named rc and defaulted to >> 0, but changed to -1 on error" to the more common "named ret and >> defaulted to -1, but changed to 0 on success". >> >> 2) Rename the "error" label to "cleanup", since the code that follows >> is executed in success cases as well as failure. >> --- >> src/util/virnetlink.c | 42 -- >> 1 file changed, 20 insertions(+), 22 deletions(-) > ACK. Just nits below. > >> @@ -253,26 +250,27 @@ int virNetlinkCommand(struct nl_msg *nl_msg, >> if (n == 0) >> virReportSystemError(ETIMEDOUT, "%s", >> _("no valid netlink response was >> received")); >> -rc = -1; >> -goto error; >> +goto cleanup; >> } >> >> -*respbuflen = nl_recv(nlhandle, &nladdr, >> - (unsigned char **)resp, NULL); >> -if (*respbuflen <= 0) { >> +len = nl_recv(nlhandle, &nladdr, (unsigned char **)resp, NULL); >> +if (len <= 0) { > Does nl_recv set errno when it returns 0? Good point - yet another existing bug that can be fixed in this patch. I'll add an "if (len == 0)" clause that prints out a plain error (since for us receiving nothing *is* an error). > >> virReportSystemError(errno, >> "%s", _("nl_recv failed")); >> -rc = -1; >> +goto cleanup; >> } >> - error: >> -if (rc == -1) { >> -VIR_FREE(*resp); >> + > [1] here more. > >> +ret = 0; >> + cleanup: >> +if (ret < 0) { >> *resp = NULL; >> *respbuflen = 0; >> +} else { >> +*respbuflen = len; > Personally, I'd like this assignment [1] Sure, I can do that. I pushed the patch with those two changes. Thanks for the attentive review! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5] migration: add support for migrateURI configuration
On Tue, May 20, 2014 at 10:00:52AM +0200, Jiri Denemark wrote: > s/migrateURI/migrateHost/ in Subject. > > On Tue, May 20, 2014 at 14:08:05 +0800, Chen Fan wrote: > > For now, we set the migration URI via command line '--migrate_uri' or > > construct the URI by looking up the dest host's hostname which could be > > solved by DNS automatically. > > > > But in cases the dest host have two or more NICs to reach, we may need to > > send the migration data over a specific NIC which is different from the > > automatically resloved one for some reason like performance, security, etc. > > thus we must explicitly specify the migrateuri in command line everytime, > > but it is too troublesome if there are many such hosts(and don't forget > > virt-manager). > > > > This patch adds a configuration file option on dest host to save the > > default value set which can be specified to a migration hostname or > > one of this host's addresses used for transferring data, thus user doesn't > > boring to specify it in command line everytime. > > > > Signed-off-by: Chen Fan > > --- > > > > v4-v5: using "migrate_host" instead of "migrate_uri" configuration. > > Yeah, that's the best solution which I wanted to suggest in a discussion > on v4 but I failed to actually send that email. Fortunately, Daniel > suggested the same :-) Although I'd call it "migration_host" to be > consistent with existing migration_address, migration_port_min, and > migration_port_max. ACK with Jiri's proposed additions. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virlog: Add stack trace log when libvirt receives fatal signal
An earlier commit(c0c8c1) Dan removed global log buffer feature entirely because of duplicate log messages. An improvement is introduced. That is dumping stack trace instead of log buffer upon libvirt crash. Signed-off-by: Zhou Yimin Signed-off-by: Wang Rui --- When libvirt receives fatal signal, the log output shows as following. Caught abort signal dumping stack trace: == start of stack trace = /usr/lib64/libvirt.so.0(+0x1d4fde)[0x7f001b1b0fde] /lib64/libpthread.so.0(+0xf800)[0x7f00182ed800] /lib64/libc.so.6(gsignal+0x35)[0x7f0017b91b55] /lib64/libc.so.6(abort+0x181)[0x7f0017b93131] /lib64/libc.so.6(+0x70e0f)[0x7f0017bcfe0f] /lib64/libc.so.6(+0x76618)[0x7f0017bd5618] /lib64/libc.so.6(+0x76c3f)[0x7f0017bd5c3f] /lib64/libc.so.6(+0x78fee)[0x7f0017bd7fee] /lib64/libc.so.6(__libc_malloc+0x77)[0x7f0017bda747] /usr/lib64/libvirt.so.0(virReallocN+0x65)[0x7f001b03c06d] /usr/lib64/libvirt.so.0(+0x7ebbc)[0x7f001b05abbc] /usr/lib64/libvirt.so.0(virFileReadLimFD+0x50)[0x7f001b05ad4a] /usr/lib64/libvirt.so.0(virFileReadAll+0xa6)[0x7f001b05c7d1] /usr/lib64/libvirt.so.0(virProcessGetStartTime+0x9a)[0x7f001b080ee9] /usr/lib64/libvirt.so.0(virNetSocketGetUNIXIdentity+0xc1)[0x7f001b1bf709] /usr/lib64/libvirt.so.0(virNetServerClientGetUNIXIdentity+0x68)[0x7f001b1b35b6] /usr/sbin/libvirtd(+0x20103)[0x7f001bb8f103] /usr/sbin/libvirtd(+0x20347)[0x7f001bb8f347] /usr/lib64/libvirt.so.0(+0x1db9bd)[0x7f001b1b79bd] /usr/lib64/libvirt.so.0(virNetServerProgramDispatch+0x209)[0x7f001b1b7dff] /usr/lib64/libvirt.so.0(+0x1d5448)[0x7f001b1b1448] /usr/lib64/libvirt.so.0(+0x1d572a)[0x7f001b1b172a] /usr/lib64/libvirt.so.0(+0xb3964)[0x7f001b08f964] /usr/lib64/libvirt.so.0(+0xb2dd0)[0x7f001b08edd0] /lib64/libpthread.so.0(+0x77f6)[0x7f00182e57f6] /lib64/libc.so.6(clone+0x6d)[0x7f0017c3a09d] == end of stack trace = src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 43 +++ src/util/virlog.c| 88 src/util/virlog.h| 2 ++ 4 files changed, 134 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3332c9..d781595 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1501,6 +1501,7 @@ virLogGetFilters; virLogGetNbFilters; virLogGetNbOutputs; virLogGetOutputs; +virLogEmergencyDumpAll; virLogLock; virLogMessage; virLogParseDefaultPriority; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 7e3fc0a..2845098 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -340,6 +340,32 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc, return 0; } +static void +virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED, +void *context ATTRIBUTE_UNUSED) +{ +struct sigaction sig_action; +int origerrno; + +origerrno = errno; +virLogEmergencyDumpAll(sig); + +/* + * If the signal is fatal, avoid looping over this handler + * by deactivating it + */ +#ifdef SIGUSR2 +if (sig != SIGUSR2) { +#endif +memset(&sig_action, 0, sizeof(sig_action)); +sig_action.sa_handler = SIG_DFL; +sigaction(sig, &sig_action, NULL); +raise(sig); +#ifdef SIGUSR2 +} +#endif +errno = origerrno; +} virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, @@ -401,6 +427,23 @@ virNetServerPtr virNetServerNew(size_t min_workers, sig_action.sa_handler = SIG_IGN; sigaction(SIGPIPE, &sig_action, NULL); +/* + * catch fatal errors to dump stack trace, also hook to USR2 for dynamic + * debugging purposes or testing + */ +sig_action.sa_sigaction = virNetServerFatalSignal; +sig_action.sa_flags = SA_SIGINFO; +sigaction(SIGFPE, &sig_action, NULL); +sigaction(SIGSEGV, &sig_action, NULL); +sigaction(SIGILL, &sig_action, NULL); +sigaction(SIGABRT, &sig_action, NULL); +#ifdef SIGBUS +sigaction(SIGBUS, &sig_action, NULL); +#endif +#ifdef SIGUSR2 +sigaction(SIGUSR2, &sig_action, NULL); +#endif + return srv; error: diff --git a/src/util/virlog.c b/src/util/virlog.c index 056950e..081b194 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #if HAVE_SYSLOG_H @@ -661,6 +662,93 @@ virLogStackTraceToFd(int fd) } static void +virLogDumpAllFD(const char *msg, int len) +{ +size_t i; +bool found = false; + +if (len <= 0) +len = strlen(msg); + +for (i = 0; i < virLogNbOutputs; i++) { +if (virLogOutputs[i].f == virLogOutputToFd) { +int fd = (intptr_t) virLogOutputs[i].data; + +if (fd >= 0) { +if (msg) +ignore_value(safewrite(fd, msg, len)); +else +virLogStackTraceToFd(fd); + +found = true; +} +} +} +i
Re: [libvirt] [PATCH] virlog: Add stack trace log when libvirt receives fatal signal
On Tue, May 20, 2014 at 08:59:37AM +, Wangrui (K) wrote: > An earlier commit(c0c8c1) Dan removed global log buffer feature entirely > because of duplicate log messages. An improvement is introduced. That is > dumping stack trace instead of log buffer upon libvirt crash. While I understand the desire here... > +virLogDumpAllFD(const char *msg, int len) > +{ > +size_t i; > +bool found = false; > + > +if (len <= 0) > +len = strlen(msg); > + > +for (i = 0; i < virLogNbOutputs; i++) { > +if (virLogOutputs[i].f == virLogOutputToFd) { > +int fd = (intptr_t) virLogOutputs[i].data; > + > +if (fd >= 0) { > +if (msg) > +ignore_value(safewrite(fd, msg, len)); > +else > +virLogStackTraceToFd(fd); > + > +found = true; > +} > +} > +} > +if (!found) { > +if (msg) > +ignore_value(safewrite(STDERR_FILENO, msg, len)); > +else > +virLogStackTraceToFd(STDERR_FILENO); This is not going to work. virLogStackTraceToFd invokes the backtrace/backtrace_symbols_fd functions which are not async signal safe. They are also not likely to be reliable to use when you have memory corruption triggering the signal. The 'abrt' program is commonly used on modern Linux distros to generate stack traces when processes crash / terminate abnormally. abrt has the added benefit that the stack traces it records include all function parameters and local variables. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] require for suggestions on support for ivshmem device
On Wed, May 14, 2014 at 08:23:21AM +, Wangrui (K) wrote: Hi, Libvirt does not support ivshmem(Inter-VM Shared Memory) device recently, thus, I would like to know if there's any plan to support it in the future? If not, I would like to contribute a serial of patches to do so. On Jan 28, Wangyufei (James) asked about this question, and Daniel replied with 2 suggestions: 1. Libvirt should be capable of configuring the guest's xml on ivshmem. 2.An ivshmem daemon is needed to run on the host to support it, libvirt is recommended to provide such a daemon. Please refer to https://www.redhat.com/archives/libvir-list/2014-January/msg01335.html for details. What I'll do later is the 1st suggestion, the 2nd one is left to be accomplished by someone else. Here is the detailed work I'll do to support configuration of the guest in libvirt: virDomainDefParseXML: parse ivshmem device xml when defining dom.xml virDomainDeviceInfoIterateInternal: iterate ivshmem devices qemuAssignDevicePCISlots: assign ivshmem device pci slots virDomainDefFormatInternal: format ivshmem device xml (Eg. virsh edit dom) virDomainDefFree: free ivshmem device def qemuBuildCommandLine: build ivshmem device command line when vm starts qemuAssignDeviceAliases: assign ivshmem device aliases when vm starts virDomainDeviceDefParse: attach and parse ivshmem device xml qemuDomainAttachDeviceConfig: attach ivshmem device xml persistently qemuDomainAttachDeviceLive: attach ivshmem device online qemuDomainDetachDeviceConfig: detach ivshmem device xml persistently qemuDomainDetachDeviceLive: detach ivshmem device online Don't forget about checking for the qemu capability and error-ing out properly in case it's not supported, you probably know you can use qemuBuildChrChardevStr() for the '-chardev' part of the commandline, various backends are supported and the code is in already. The idea looks good, it would be nice improvement to have. About the daemon, you mean it would be another daemon we have in the repo like virtlockd, I suppose. There are two ways to use ivshmem with qemu (please refer to http://qemu.weilnetz.de/qemu-doc.html#pcsys_005fother_005fdevs ): 1.Guests map a POSIX shared memory region into the guest as a PCI device that enables zero-copy communication to the application level of the guests, The basic syntax is: qemu-system-i386-device ivshmem, size = [, shm = ] 2.If desired, interrupts can be sent between guest VMs accessing the same shared memory region. Interrupt support requires using a shared memory server and using a chardev socket to connect to it. An example syntax when using the shared memory server is: qemu-system-i386-device ivshmem, size = [, chardev = ] [, msi = on] [, ioeventfd = on] [, vectors = n] [, role = peer | master] qemu-system-i386-chardev socket, path = , id = The respective xml configuration for the above 2 qemu command lines are shown as below: Example1: automatically attach device with KVM NOTE: "size" means ivshmem size in unit MB, "name" means shm name "role" is optional, it may be set to "master" or "peer", the default one is "master" What do these roles mean, I mean what's the difference between master and peer and why is it only used with the chardev? Does it mean master can only send interrupts or...? Just curious. Example2: manually attach device with static PCI slot 4 requested Example3: automatically attach device with KVM NOTE: "path" means shared memory socket path which is set by the daemon. " source mode " and "type" is similar with vmchannel. I'm looking forward to your suggestions, thank you very much. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virlog: Add stack trace log when libvirt receives fatal signal
On Tue, May 20, 2014 at 10:05:19AM +0100, Daniel P. Berrange wrote: On Tue, May 20, 2014 at 08:59:37AM +, Wangrui (K) wrote: An earlier commit(c0c8c1) Dan removed global log buffer feature entirely because of duplicate log messages. An improvement is introduced. That is dumping stack trace instead of log buffer upon libvirt crash. While I understand the desire here... +virLogDumpAllFD(const char *msg, int len) +{ +size_t i; +bool found = false; + +if (len <= 0) +len = strlen(msg); + +for (i = 0; i < virLogNbOutputs; i++) { +if (virLogOutputs[i].f == virLogOutputToFd) { +int fd = (intptr_t) virLogOutputs[i].data; + +if (fd >= 0) { +if (msg) +ignore_value(safewrite(fd, msg, len)); +else +virLogStackTraceToFd(fd); + +found = true; +} +} +} +if (!found) { +if (msg) +ignore_value(safewrite(STDERR_FILENO, msg, len)); +else +virLogStackTraceToFd(STDERR_FILENO); This is not going to work. virLogStackTraceToFd invokes the backtrace/backtrace_symbols_fd functions which are not async signal safe. They are also not likely to be reliable to use when you have memory corruption triggering the signal. The 'abrt' program is commonly used on modern Linux distros to generate stack traces when processes crash / terminate abnormally. abrt has the added benefit that the stack traces it records include all function parameters and local variables. And if you can't (or don't want to) have abrt then it is pretty easy to make a self-created solution using kernel.core_pattern in /etc/sysctl.conf Regards, Daniel signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cleanup: clamp max info with MIN().
On Thu, May 15, 2014 at 08:03:09PM +0900, Dongsheng Yang wrote: Rather than using a open coded implementation, this patch use MIN macro to clamp infomation to allowed maxmum. Sorry to say that, but I don't find it as readable as before. At first, the idea with CLAMP was a nice improvement in case of both ends of the range being utilized, but I don't see any added value in this particular patch. I was waiting if somebody expresses their opinion on this patch, but no response yet. If there is particular need for this patch (or it helps with something I don't see), let me know, but if not then I'd leave it as is. Mainly because some future patches will most likely not use MIN() and we would have another inconsistency in the code. Martin Signed-off-by: Dongsheng Yang --- src/qemu/qemu_driver.c | 18 ++ src/test/test_driver.c | 10 +++--- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 52ca47c..b3782dd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4542,12 +4542,9 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, goto cleanup; maxcpu = maplen * 8; -if (maxcpu > hostcpus) -maxcpu = hostcpus; -/* Clamp to actual number of vcpus */ -if (ncpumaps > targetDef->vcpus) -ncpumaps = targetDef->vcpus; +maxcpu = MIN(maxcpu, hostcpus); +ncpumaps = MIN(ncpumaps, targetDef->vcpus); if (ncpumaps < 1) { goto cleanup; @@ -4786,8 +4783,8 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, goto cleanup; maxcpu = maplen * 8; -if (maxcpu > hostcpus) -maxcpu = hostcpus; + +maxcpu = MIN(maxcpu, hostcpus); /* initialize cpumaps */ memset(cpumaps, 0xff, maplen); @@ -4852,12 +4849,9 @@ qemuDomainGetVcpus(virDomainPtr dom, goto cleanup; maxcpu = maplen * 8; -if (maxcpu > hostcpus) -maxcpu = hostcpus; -/* Clamp to actual number of vcpus */ -if (maxinfo > priv->nvcpupids) -maxinfo = priv->nvcpupids; +maxcpu = MIN(maxcpu, hostcpus); +maxinfo = MIN(maxinfo, priv->nvcpupids); if (maxinfo >= 1) { if (info != NULL) { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 37756e7..4e591f2 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2773,12 +2773,9 @@ static int testDomainGetVcpus(virDomainPtr domain, hostcpus = VIR_NODEINFO_MAXCPUS(privconn->nodeInfo); maxcpu = maplen * 8; -if (maxcpu > hostcpus) -maxcpu = hostcpus; -/* Clamp to actual number of vcpus */ -if (maxinfo > privdom->def->vcpus) -maxinfo = privdom->def->vcpus; +maxcpu = MIN(maxcpu, hostcpus); +maxinfo = MIN(maxinfo, privdom->def->vcpus); /* Populate virVcpuInfo structures */ if (info != NULL) { @@ -2858,8 +2855,7 @@ static int testDomainPinVcpu(virDomainPtr domain, privmaplen = VIR_CPU_MAPLEN(hostcpus); maxcpu = maplen * 8; -if (maxcpu > hostcpus) -maxcpu = hostcpus; +maxcpu = MIN(maxcpu, hostcpus); privcpumap = VIR_GET_CPUMAP(privdomdata->cpumaps, privmaplen, vcpu); memset(privcpumap, 0, privmaplen); -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cleanup: clamp max info with MIN().
On Tue, May 20, 2014 at 11:28:02AM +0200, Martin Kletzander wrote: > On Thu, May 15, 2014 at 08:03:09PM +0900, Dongsheng Yang wrote: > >Rather than using a open coded implementation, > >this patch use MIN macro to clamp infomation > >to allowed maxmum. > > > > Sorry to say that, but I don't find it as readable as before. At > first, the idea with CLAMP was a nice improvement in case of both ends > of the range being utilized, but I don't see any added value in this > particular patch. > > I was waiting if somebody expresses their opinion on this patch, but > no response yet. If there is particular need for this patch (or it > helps with something I don't see), let me know, but if not then I'd > leave it as is. Mainly because some future patches will most likely > not use MIN() and we would have another inconsistency in the code. I tend to agree - I think it is easier to understand the code when not using the macro here. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: domain events support
On Wed, Apr 23, 2014 at 07:50:09PM +0400, Roman Bogorodskiy wrote: > Support events for these functions: > > - domainDefineXML > - domainUndefine > - domainCreate{WithFlags,XML} > - domainDestroy > --- > src/bhyve/bhyve_driver.c | 88 > +++- > src/bhyve/bhyve_utils.h | 3 ++ > 2 files changed, 90 insertions(+), 1 deletion(-) ACK, with one tweak > @@ -727,8 +746,16 @@ bhyveDomainCreateWithFlags(virDomainPtr dom, > VIR_DOMAIN_RUNNING_BOOTED, > start_flags); > > +if (ret == 0) { > +event = virDomainEventLifecycleNewFromObj(vm, > + VIR_DOMAIN_EVENT_STARTED, > + > VIR_DOMAIN_EVENT_STARTED_BOOTED); > +} Nit-pick - no need for curly brackets here Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6] migration: add support for migrateHost configuration
For now, we set the migration URI via command line '--migrate_uri' or construct the URI by looking up the dest host's hostname which could be solved by DNS automatically. But in cases the dest host have two or more NICs to reach, we may need to send the migration data over a specific NIC which is different from the automatically resloved one for some reason like performance, security, etc. thus we must explicitly specify the migrateuri in command line everytime, but it is too troublesome if there are many such hosts(and don't forget virt-manager). This patch adds a configuration file option on dest host to save the default value set which can be specified to a migration hostname or one of this host's addresses used for transferring data, thus user doesn't boring to specify it in command line everytime. Signed-off-by: Chen Fan --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 9 - src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_migration.c | 21 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index e985d22..e7db7fe 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -84,6 +84,7 @@ module Libvirtd_qemu = let network_entry = str_entry "migration_address" | int_entry "migration_port_min" | int_entry "migration_port_max" + | str_entry "migration_host" let log_entry = bool_entry "log_timestamp" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 42f812d..1063625 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -450,12 +450,19 @@ #seccomp_sandbox = 1 - # Override the listen address for all incoming migrations. Defaults to # 0.0.0.0, or :: if both host and qemu are capable of IPv6. #migration_address = "127.0.0.1" +# The default hostname or IP address which will be used by a migration +# source for transferring migration data to this host. The migration +# source has to be able to resolve this hostname and connect to it so +# setting "localhost" will not work. By default, the host's configured +# hostname is used. +#migration_host = "host.example.com" + + # Override the port range used for incoming migrations. # # Minimum must be greater than 0, however when QEMU is not running as root, diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 400c99c..f273056 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -576,6 +576,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox); +GET_VALUE_STR("migration_host", cfg->migrateHost); GET_VALUE_STR("migration_address", cfg->migrationAddress); GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5d2983a..78b08e5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -163,6 +163,7 @@ struct _virQEMUDriverConfig { int seccompSandbox; +char *migrateHost; /* The default for -incoming */ char *migrationAddress; int migrationPortMin; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0df1a6..d6271fb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2640,6 +2640,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, int ret = -1; virURIPtr uri = NULL; bool well_formed_uri = true; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +const char *migrateHost = cfg->migrateHost; VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " @@ -2653,8 +2655,9 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, /* The URI passed in may be NULL or a string "tcp://somehostname:port". * * If the URI passed in is NULL then we allocate a port number - * from our pool of port numbers and return a URI of - * "tcp://ourhostname:port". + * from our pool of port numbers, and if the migrateHost is configured, + * we return a URI of "tcp://migrateHost:port", otherwise return a URI + * of "tcp://ourhostname:port". * * If the URI passed in is not NULL then we try to parse out the * port number and use that (note that the hostname is assumed @@ -2664,8 +2667,17 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) goto cleanup; -if ((hostname = virGetHostname()) == NULL) -goto cleanup; +if (migrateHost != NULL) { +if (virSocketAddrIsNumeric(migrateHost) && +virSocketAddrParse(NULL, migrateHost, AF_UNSPEC) < 0) +goto cleanup; + + if (VIR_STRDUP(hostname
Re: [libvirt] [PATCH v5] migration: add support for migrateURI configuration
On Tue, May 20, 2014 at 09:43:03 +0100, Daniel Berrange wrote: > On Tue, May 20, 2014 at 10:00:52AM +0200, Jiri Denemark wrote: > > s/migrateURI/migrateHost/ in Subject. > > > > On Tue, May 20, 2014 at 14:08:05 +0800, Chen Fan wrote: > > > For now, we set the migration URI via command line '--migrate_uri' or > > > construct the URI by looking up the dest host's hostname which could be > > > solved by DNS automatically. > > > > > > But in cases the dest host have two or more NICs to reach, we may need to > > > send the migration data over a specific NIC which is different from the > > > automatically resloved one for some reason like performance, security, > > > etc. > > > thus we must explicitly specify the migrateuri in command line everytime, > > > but it is too troublesome if there are many such hosts(and don't forget > > > virt-manager). > > > > > > This patch adds a configuration file option on dest host to save the > > > default value set which can be specified to a migration hostname or > > > one of this host's addresses used for transferring data, thus user doesn't > > > boring to specify it in command line everytime. > > > > > > Signed-off-by: Chen Fan > > > --- > > > > > > v4-v5: using "migrate_host" instead of "migrate_uri" configuration. > > > > Yeah, that's the best solution which I wanted to suggest in a discussion > > on v4 but I failed to actually send that email. Fortunately, Daniel > > suggested the same :-) Although I'd call it "migration_host" to be > > consistent with existing migration_address, migration_port_min, and > > migration_port_max. > > ACK with Jiri's proposed additions. OK, I applied the changes and push this patch. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6] migration: add support for migrateHost configuration
On Tue, May 20, 2014 at 18:26:04 +0800, Chen Fan wrote: > For now, we set the migration URI via command line '--migrate_uri' or > construct the URI by looking up the dest host's hostname which could be > solved by DNS automatically. > > But in cases the dest host have two or more NICs to reach, we may need to > send the migration data over a specific NIC which is different from the > automatically resloved one for some reason like performance, security, etc. > thus we must explicitly specify the migrateuri in command line everytime, > but it is too troublesome if there are many such hosts(and don't forget > virt-manager). > > This patch adds a configuration file option on dest host to save the > default value set which can be specified to a migration hostname or > one of this host's addresses used for transferring data, thus user doesn't > boring to specify it in command line everytime. > > Signed-off-by: Chen Fan > --- > src/qemu/libvirtd_qemu.aug | 1 + > src/qemu/qemu.conf | 9 - > src/qemu/qemu_conf.c | 1 + > src/qemu/qemu_conf.h | 1 + > src/qemu/qemu_migration.c | 21 + > src/qemu/test_libvirtd_qemu.aug.in | 1 + > 6 files changed, 29 insertions(+), 5 deletions(-) Oops, I already took v5, applied the changes I suggested and Daniel ACKed and pushed the result. So this is already pushed now. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
On 05/20/2014 01:25 AM, hong-hua@freescale.com wrote: > Hi Cole, > > Thanks for the comments. > Exactly there were already test cases for both pseries and ppce500 machines. > For example, > 1) qemuxml2argv-pseries-basic.args: '-chardev spapr-vty' for pseries. > 2) qemuxml2argv-ppc-dtb.args: '-serial pty' for ppce500. > If there's already test cases, then what does the patch actually change? What I'm suggesting is to add a test case that would fail before this patch, and succeed afterwards, to demonstrate what is actually changing. - Cole > > >> -Original Message- >> From: Cole Robinson [mailto:crobi...@redhat.com] >> Sent: Monday, May 19, 2014 9:40 PM >> To: Yin Olivia-R63875; libvir-list@redhat.com; zhlci...@linux.vnet.ibm.com >> Subject: Re: [PATCH] qemu: Fix specifying char devs for PPC >> >> On 05/19/2014 03:41 AM, hong-hua@freescale.com wrote: >>> Hi Cole, >>> >>> This is a patch similar with your previous patch to fix for ARM. >>> Do you have any comments on it? >>> >>> Cindy, >>> Since you are the main contributor to QEMU driver on PPC, I'll also >> appreciate your comments. >>> >>> Best Regards, >>> Olivia >>> >> >> Patch looks fine, but it should add a qemuxml2argv test case to validate >> that it actually works. My original patch added test cases later in the >> patch series, see 54a77c6df3c483864463f602c4c6f435d50bd79e >> >> - Cole >> -Original Message- From: Yin Olivia-R63875 Sent: Friday, May 16, 2014 8:38 AM To: Yin Olivia-R63875; libvir-list@redhat.com Subject: RE: [PATCH] qemu: Fix specifying char devs for PPC Ping. This is a patch similar with ARM platforms. http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc96 8171c5 84ec2edcfdcb8fadde Right now on ppce500, chardev is not supported for the serial console. So it uses the the legacy -serial option. Best Regards, Olivia > -Original Message- > From: Olivia Yin [mailto:hong-hua@freescale.com] > Sent: Wednesday, May 14, 2014 6:47 PM > To: libvir-list@redhat.com > Cc: Yin Olivia-R63875 > Subject: [PATCH] qemu: Fix specifying char devs for PPC > > QEMU ppce500 board uses the old style -serial options. > > Other PPC boards don't give any way to explicitly wire in a -chardev > except pseries which uses -device spapr-vty with -chardev. > > --- > src/qemu/qemu_capabilities.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c > b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, > !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) > return false; > > -if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != > VIR_ARCH_AARCH64)) > +if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != > VIR_ARCH_AARCH64) > + && (def->os.arch != VIR_ARCH_PPC) && (def->os.arch != > +VIR_ARCH_PPC64)) > return true; > > /* This may not be true for all ARM machine types, but at least > * the only supported non-virtio serial devices of vexpress and > versatile > - * don't have the -chardev property wired up. */ > + * don't have the -chardev property wired up. > + * For PPC machines, only pserial need -device spapr-vty with > + -chardev */ > return (chr->info.type == > VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO > || > (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && > - chr->targetType == > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); > + chr->targetType == > + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) > || > +(chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && > + chr->info.type == > + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)); > } > -- > 1.8.5 >>> > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Various snapshot fixes for qemu
Peter Krempa (4): qemu: snapshot: Use typecasted switch in qemuDomainSnapshotPrepare() qemu: snapshot: Forbid partial internal snapshots qemu: snapshot: Forbid empty snapshots qemu: snapshot: Fix return value of external checkpoint with no disks src/qemu/qemu_driver.c | 36 +--- 1 file changed, 25 insertions(+), 11 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] qemu: snapshot: Forbid empty snapshots
If neither disks nor memory are selected for snapshot we'd record metadata in case of external snapshot and do a disk snapshot in case of external disk snapshot. Forbid this as it doesn't make much sense. --- Notes: There's a slightly usable case where this would be used to backup domain's config. src/qemu/qemu_driver.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e6b6648..6442b3f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12703,6 +12703,13 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, } } +if (!found_internal && !external && +def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nothing selected for snapshot")); +goto cleanup; +} + /* internal snapshot requires a disk image to store the memory image to and * also disks can't be excluded from a internal snapshot*/ if ((def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && !found_internal) || -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] qemu: snapshot: Use typecasted switch in qemuDomainSnapshotPrepare()
Convert the switch to a typecasted value so that the compiler tracks additions for us. --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8a0029..aa07ef6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12635,7 +12635,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, virDomainSnapshotDiskDefPtr disk = &def->disks[i]; virDomainDiskDefPtr dom_disk = vm->def->disks[i]; -switch (disk->snapshot) { +switch ((virDomainSnapshotLocation) disk->snapshot) { case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: found_internal = true; @@ -12692,7 +12692,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, break; case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: -default: +case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected code path")); goto cleanup; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
When doing an external checkpoint of a VM with no disk selected we'd return failure but not set error code. This was a result of ret not being set to 0 during walking of the disk array. Rework early failure checking to avoid the problem. Fixes the following symptom (or without --diskspec for diskless VMs) $ virsh snapshot-create-as snapshot-test --memspec /tmp/asdf --diskspec hda,snapshot=no error: An error occurred, but the cause is unknown --- src/qemu/qemu_driver.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6442b3f..460c680 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13010,17 +13010,17 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virJSONValuePtr actions = NULL; -int ret = -1; +int ret = 0; size_t i; bool persist = false; bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; -virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +virQEMUDriverConfigPtr cfg = NULL; virErrorPtr orig_err = NULL; if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); -goto cleanup; +return -1; } if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) { @@ -13030,9 +13030,11 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("live disk snapshot not supported with this " "QEMU binary")); -goto cleanup; +return -1; } +cfg = virQEMUDriverGetConfig(driver); + /* No way to roll back if first disk succeeds but later disks * fail, unless we have transaction support. * Based on earlier qemuDomainSnapshotPrepare, all -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] qemu: snapshot: Forbid partial internal snapshots
qemu's savevm command does a snapshot of all non readonly disks of a VM. Libvirt though allowed disabling snapshot for certain disk of a VM. --- src/qemu/qemu_driver.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aa07ef6..e6b6648 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12621,6 +12621,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, bool reuse = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; bool atomic = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0; bool found_internal = false; +bool forbid_internal = false; int external = 0; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -12689,6 +12690,9 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, break; case VIR_DOMAIN_SNAPSHOT_LOCATION_NONE: +/* Remember seeing a disk that has snapshot disabled */ +if (!dom_disk->readonly) +forbid_internal = true; break; case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: @@ -12699,12 +12703,13 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, } } -/* internal snapshot requires a disk image to store the memory image to */ -if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && -!found_internal) { +/* internal snapshot requires a disk image to store the memory image to and + * also disks can't be excluded from a internal snapshot*/ +if ((def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && !found_internal) || +(found_internal && forbid_internal)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("internal checkpoints require at least " - "one disk to be selected for snapshot")); + _("internal snapshots and checkpoints require all " + "disks to be selected for snapshot")); goto cleanup; } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: snapshot: Forbid empty snapshots
On 05/20/2014 03:36 PM, Peter Krempa wrote: > If neither disks nor memory are selected for snapshot we'd record > metadata in case of external snapshot and do a disk snapshot in case of > external disk snapshot. Forbid this as it doesn't make much sense. s/external/internal/ > --- > > Notes: > There's a slightly usable case where this would be used to backup > domain's config. > > src/qemu/qemu_driver.c | 7 +++ > 1 file changed, 7 insertions(+) Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Various snapshot fixes for qemu
On 05/20/2014 03:35 PM, Peter Krempa wrote: > Peter Krempa (4): > qemu: snapshot: Use typecasted switch in qemuDomainSnapshotPrepare() > qemu: snapshot: Forbid partial internal snapshots > qemu: snapshot: Forbid empty snapshots > qemu: snapshot: Fix return value of external checkpoint with no disks > > src/qemu/qemu_driver.c | 36 +--- > 1 file changed, 25 insertions(+), 11 deletions(-) > ACK series. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: snapshot: Forbid partial internal snapshots
On 05/20/2014 03:36 PM, Peter Krempa wrote: > qemu's savevm command does a snapshot of all non readonly disks of a VM. > Libvirt though allowed disabling snapshot for certain disk of a VM. > --- > src/qemu/qemu_driver.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index aa07ef6..e6b6648 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -12699,12 +12703,13 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, > } > } > > -/* internal snapshot requires a disk image to store the memory image to > */ > -if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && > -!found_internal) { > +/* internal snapshot requires a disk image to store the memory image to > and > + * also disks can't be excluded from a internal snapshot*/ s/a internal snapshot/an internal snapshot / > +if ((def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && > !found_internal) || > +(found_internal && forbid_internal)) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("internal checkpoints require at least " > - "one disk to be selected for snapshot")); > + _("internal snapshots and checkpoints require all " > + "disks to be selected for snapshot")); > goto cleanup; > } > Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] qemu: snapshot: Use typecasted switch in qemuDomainSnapshotPrepare()
On 05/20/2014 07:35 AM, Peter Krempa wrote: > Convert the switch to a typecasted value so that the compiler tracks > additions for us. > --- > src/qemu/qemu_driver.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: snapshot: Forbid partial internal snapshots
On 05/20/2014 07:36 AM, Peter Krempa wrote: > qemu's savevm command does a snapshot of all non readonly disks of a VM. > Libvirt though allowed disabling snapshot for certain disk of a VM. > --- > src/qemu/qemu_driver.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) I think this may even be a regression introduced when we added checkpoint snapshots, because I seem to recall trying to prevent just this back when doing external disk snapshots. But I didn't bother to crawl git history to confirm or deny my suspicion. > @@ -12699,12 +12703,13 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, > } > } > > -/* internal snapshot requires a disk image to store the memory image to > */ > -if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && > -!found_internal) { > +/* internal snapshot requires a disk image to store the memory image to > and s/to and/to, and/ > + * also disks can't be excluded from a internal snapshot*/ s/a internal/an internal/ > +if ((def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && > !found_internal) || > +(found_internal && forbid_internal)) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("internal checkpoints require at least " > - "one disk to be selected for snapshot")); > + _("internal snapshots and checkpoints require all " > + "disks to be selected for snapshot")); > goto cleanup; > } > ACK with the comment grammar fixes -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: snapshot: Forbid empty snapshots
On 05/20/2014 07:36 AM, Peter Krempa wrote: > If neither disks nor memory are selected for snapshot we'd record > metadata in case of external snapshot and do a disk snapshot in case of > external disk snapshot. Forbid this as it doesn't make much sense. > --- > > Notes: > There's a slightly usable case where this would be used to backup > domain's config. Hmm. Down the road, when we start integrating chain operations (pull, commit) more closely with snapshots, we may hit the case where a blockpull goes through and modifies a snapshot to record that the disk snapshot taken at that point of time was invalidated by the chain operation. Perhaps we could still leave the empty snapshot in the list of snapshots, and if so, then this code should also allow creation of empty snapshots. But I tend to agree that it sounds rather pointless to track empty snapshots, so I'd rather have chain operations delete snapshots that are otherwise rendered empty, and agree with the approach of this patch in forbidding an empty snapshot. Someone wanting to preserve configuration can use virsh dumpxml. > +++ b/src/qemu/qemu_driver.c > @@ -12703,6 +12703,13 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, > } > } > > +if (!found_internal && !external && > +def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) { > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("nothing selected for snapshot")); > +goto cleanup; > +} ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/2014 07:36 AM, Peter Krempa wrote: > When doing an external checkpoint of a VM with no disk selected we'd > return failure but not set error code. This was a result of ret not > being set to 0 during walking of the disk array. > > Rework early failure checking to avoid the problem. > > Fixes the following symptom (or without --diskspec for diskless VMs) > > $ virsh snapshot-create-as snapshot-test --memspec /tmp/asdf --diskspec > hda,snapshot=no > error: An error occurred, but the cause is unknown > --- Technically, this is equivalent to a 'virsh save'. Would it be better to avoid an error altogether and allow the operation as a longhand spelling of the same action? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/14 16:28, Eric Blake wrote: > On 05/20/2014 07:36 AM, Peter Krempa wrote: >> When doing an external checkpoint of a VM with no disk selected we'd >> return failure but not set error code. This was a result of ret not >> being set to 0 during walking of the disk array. >> >> Rework early failure checking to avoid the problem. >> >> Fixes the following symptom (or without --diskspec for diskless VMs) >> >> $ virsh snapshot-create-as snapshot-test --memspec /tmp/asdf --diskspec >> hda,snapshot=no >> error: An error occurred, but the cause is unknown >> --- > > Technically, this is equivalent to a 'virsh save'. Would it be better > to avoid an error altogether and allow the operation as a longhand > spelling of the same action? > virsh save kills the VM after it finishes. This allows to preserve the VM running and also allows snapshots of diskless VMs. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] PCI: Introduce new device binding path using pci_dev.driver_override
The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above existing process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub > /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 > /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 > /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device: echo > /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 > /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 > /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (override driver = "none") or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson Cc: Greg Kroah-Hartman --- v3: kfree() override buffer on device release, noted by Alex Graf v2: Use strchr() as suggested by Guenter Roeck and adopted by the platform driver version of this same interface. Documentation/ABI/testing/sysfs-bus-pci | 21 drivers/pci/pci-driver.c| 25 +-- drivers/pci/pci-sysfs.c | 40 +++ drivers/pci/probe.c |1 + include/linux/pci.h |1 + 5 files changed, 85 insertions(+), 3 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci index a3c5a66..898ddc4 100644 --- a/Documentation/ABI/testing/sysfs-bus-pci +++ b/Documentation/ABI/testing/sysfs-bus-pci @@ -250,3 +250,24 @@ Description: valid. For example, writing a 2 to this file when sriov_numvfs is not 0 and not 2 already will return an error. Writing a 10 when the value of sriov_totalvfs is 8 will return an error. + +What: /sys/bus/pci/devices/.../driver_override +Date: April 2014 +Contact: Alex Williamson +Description: + This file allows the driver for a device to be specified which + will override standard static and dynamic ID matching. When + specified, only a driver with a name matching the value written + to driver_override will have an opportunity to bind to the + device. The override is specified by writing a string to the + driver_override file (echo pci-stub > driver_override) and + may be cleared with an empty string (echo > driver_override). + This returns the device to standard matching rules binding. + Writing to driver_override does not automatically unbind the + device from its current driver or make any attempt to + automatically load the specified driver. If no driver with a + matching name is currently loaded in the kernel, the device + will not bind to any driver. This also allows devices to + opt-out of driver binding using a driver_override name such as + "none". Only a single driver may be specified in the override, + there is no support for parsing delimiters. diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index d911e0c..4393c12 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -216,6 +216,13 @@ const struct pci_device_id *pci_match_id(const struct pci_device_id *ids, return NULL; } +static const struct pci_device_id pci_device_id_any = { + .vendor = PCI_ANY_ID, + .device = PCI_ANY_ID, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID, +}; + /** * pci_match_device - Tell if a PCI device s
[libvirt] Job Control, Google Summer of Code
Hello everyone, My name is Tucker DiNapoli and I'm going to be working on libvirt this summer for the google summer of code. My project is to implement job control in the storage driver. In order to do that I am first going to create a unified api for job control in libvirt (as it is currently implemented seperately in both the qemu and libxl drivers). I'm currently working on the api for job control and I have some ideas that I'll post on the mailing list soon. If anyone has any ideas or requests re job control I'd be happy to hear them. I'm on the #virt irc channel as tuckerD, I look forward to working on libvrt this summer. Tucker Dinapoli -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Job Control, Google Summer of Code
On Tue, May 20, 2014 at 11:03:46AM -0400, Tucker DiNapoli wrote: Hello everyone, My name is Tucker DiNapoli and I'm going to be working on libvirt this summer for the google summer of code. My project is to implement job control in the storage driver. In order to do that I am first going to create a unified api for job control in libvirt (as it is currently implemented seperately in both the qemu and libxl drivers). I'm currently working on the api for job control and I have some ideas that I'll post on the mailing list soon. If anyone has any ideas or requests re job control I'd be happy to hear them. I'm on the #virt irc channel as tuckerD, I look forward to working on libvrt this summer. Tucker Dinapoli Welcome, Tucker. We're glad to have you and wish you good luck on abstracting that job control (and we'll see after that ;-) ). Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/2014 08:29 AM, Peter Krempa wrote: > On 05/20/14 16:28, Eric Blake wrote: >> On 05/20/2014 07:36 AM, Peter Krempa wrote: >>> When doing an external checkpoint of a VM with no disk selected we'd >>> return failure but not set error code. This was a result of ret not >>> being set to 0 during walking of the disk array. >>> >>> Rework early failure checking to avoid the problem. >>> >>> Fixes the following symptom (or without --diskspec for diskless VMs) >>> >>> $ virsh snapshot-create-as snapshot-test --memspec /tmp/asdf --diskspec >>> hda,snapshot=no >>> error: An error occurred, but the cause is unknown >>> --- >> >> Technically, this is equivalent to a 'virsh save'. Would it be better >> to avoid an error altogether and allow the operation as a longhand >> spelling of the same action? >> > > virsh save kills the VM after it finishes. This allows to preserve the > VM running and also allows snapshots of diskless VMs. Adding an option to 'virsh save' to not kill the guest doesn't make much sense in general, but might make sense for a diskless VM. Meanwhile, don't we already have the option to kill or keep the guest alive after an external memory snapshot? So I'm not quite sure where you are heading with this patch - is the idea that we WANT to allow an external checkpoint with no disks, and this is fixing a bug that gave an unknown error, or is this a case where we want to forbid diskless external checkpoints, and you are just improving the error message? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/2014 10:16 AM, Eric Blake wrote: > So I'm not quite sure where you are heading with this patch - is the > idea that we WANT to allow an external checkpoint with no disks, and > this is fixing a bug that gave an unknown error, or is this a case where > we want to forbid diskless external checkpoints, and you are just > improving the error message? More concretely, I _want_ to allow a memory-only external snapshot (particularly of a diskless VM). If this patch is fixing a bogus error message into allowing that case, then please touch up the commit message to make that a bit more clear, and I'm fine with including the patch (my initial read was that this patch is just improving error message quality but still leaving it an error, but on reflection it looks like I mis-read the intent because of confusion about the commit message). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Job Control, Google Summer of Code
On Tue, May 20, 2014 at 11:03:46AM -0400, Tucker DiNapoli wrote: > Hello everyone, > My name is Tucker DiNapoli and I'm going to be working on libvirt this > summer for the google summer of code. My project is to implement job > control in the storage driver. In order to do that I am first going to > create a unified api for job control in libvirt (as it is currently > implemented seperately in both the qemu and libxl drivers). I'm currently > working on the api for job control and I have some ideas that I'll post on > the mailing list soon. If anyone has any ideas or requests re job control > I'd be happy to hear them. I'm on the #virt irc channel as tuckerD, I look > forward to working on libvrt this summer. Welcome to the libvirt community - its nice to have you on board for GSoC and hopefully beyond... Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/2014 10:16 AM, Eric Blake wrote: > > Adding an option to 'virsh save' to not kill the guest doesn't make much > sense in general, because memory state without matching disk state can't be used to resume a guest. > but might make sense for a diskless VM. Meanwhile, > don't we already have the option to kill or keep the guest alive after > an external memory snapshot? Here, I was thinking of VIR_DOMAIN_SNAPSHOT_CREATE_HALT. Thus, 'virsh save $dom $file' appears to be shorthand for diskless 'virsh snapshot-create-as $dom --halt --memspec $file' -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: domain events support
Daniel P. Berrange wrote: > On Wed, Apr 23, 2014 at 07:50:09PM +0400, Roman Bogorodskiy wrote: > > Support events for these functions: > > > > - domainDefineXML > > - domainUndefine > > - domainCreate{WithFlags,XML} > > - domainDestroy > > --- > > src/bhyve/bhyve_driver.c | 88 > > +++- > > src/bhyve/bhyve_utils.h | 3 ++ > > 2 files changed, 90 insertions(+), 1 deletion(-) > > ACK, with one tweak > > > @@ -727,8 +746,16 @@ bhyveDomainCreateWithFlags(virDomainPtr dom, > > VIR_DOMAIN_RUNNING_BOOTED, > > start_flags); > > > > +if (ret == 0) { > > +event = virDomainEventLifecycleNewFromObj(vm, > > + VIR_DOMAIN_EVENT_STARTED, > > + > > VIR_DOMAIN_EVENT_STARTED_BOOTED); > > +} > > Nit-pick - no need for curly brackets here Pushed with this fix and s/1.2.4/1.2.5/ for the new functions; thanks! Roman Bogorodskiy pgpdas_Dn0_wJ.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Job Control, Google Summer of Code
On 05/20/2014 09:03 AM, Tucker DiNapoli wrote: > Hello everyone, > My name is Tucker DiNapoli and I'm going to be working on libvirt this > summer for the google summer of code. My project is to implement job > control in the storage driver. In order to do that I am first going to > create a unified api for job control in libvirt (as it is currently > implemented seperately in both the qemu and libxl drivers). I'm currently > working on the api for job control and I have some ideas that I'll post on > the mailing list soon. If anyone has any ideas or requests re job control > I'd be happy to hear them. I'm on the #virt irc channel as tuckerD, I look > forward to working on libvrt this summer. Welcome! I definitely have some ideas on the matter: https://www.redhat.com/archives/libvir-list/2014-February/msg01128.html and in notes I've taken from off-list communications: >> For that matter, we need to overhaul the job support. Right now, >> virDomainBlockJob* assumes you can only have one outstanding job at a >> time, which interferes with the notion of running fleecing from two >> points in time. Similarly, there can only be one virDomainGetJobInfo >> active. What we really need is a notion of a job id, a way to query the >> job id most recently created by any existing job-related API, and a way >> to query job status based on id rather than only the most-recently >> started job. I'd _really_ like to see a common notion of a 'job id' that EVERY job (whether domain-level, like migration; or block-level, like commit/pull/rebase; or storage-level, like your new proposed storage jobs) shares a common job namespace. The job id is a positive integer. Existing APIs will have to be retrofitted into the new job id notion; any action that starts a long-running job that currently returns 0 on success could be changed to return a positive job id; or we may need a new API that queries the notion of the 'current job' (the job most recently started) or even to set the 'current job' to a different job id. We'll need new API for querying a job by id, and to be most portable, we should do job reporting via virTypedParameter (virDomainGetJobInfo and virDomainGetBlockJobInfo are hardcoded into returning a struct, so they are non-extensible; virDomainGetJobStats almost did it right, except that the user has to call it twice, once to learn how large to allocate, and again to pass in pre-allocated memory - the ideal API would allocate the memory on a single call). The fact that many jobs are exclusive (only one domain job at a time, and only one block job per disk) is currently inherent in the design of not having a job id; but we eventually WANT to allow parallel jobs. So I'm looking forward to reviewing your proposals. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [bug] problem with python interface, dom.vcpus() cpu info doesn't match cpu map
Hi, I was playing around with vcpupin and emulatorpin and managed to get into a strange state. From within python I get the following: (Pdb) dom = self._lookup_by_name(instance.name) (Pdb) dom.vcpus() ([(0, 1, 597000L, 2), (1, 1, 458000L, 3)], [(False, False, True, False), (False, False, True, False)]) The problem is that the cpuinfo for the second vcpu has it running on physical cpu 3, even though the affinity mask (within python and from taskset) says it can only run on physical cpu 2. The VM in question was originally started up running on pysical cpus 2 and 3, then I used the vcpupin/emulatorpin commands to only use physical cpu 2. Anyone got any ideas? I'm using libvirt-1.1.2 and libvirt-python-1.1.2 on a 3.4.82 kernel. Some more data: I have one VM running, taskset shows affinity as follows: root@compute-0:~# taskset -pac 7680 pid 7680's current affinity list: 2 pid 7681's current affinity list: 1-3 pid 7683's current affinity list: 2 pid 7684's current affinity list: 0 "virsh" has the following view: root@compute-0:~# virsh list IdName State 3 instance-001d running root@compute-0:~# virsh emulatorpin 3 emulator: CPU Affinity -- *: 2 root@compute-0:~# virsh vcpupin 3 VCPU: CPU Affinity -- 0: 2 1: 2 Thanks, Chris -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Job Control, Google Summer of Code
Success Tucker. I am also working on snapshot support on the libvirt xenlight driver as part of GSoC On Tue, May 20, 2014 at 7:40 PM, Eric Blake wrote: > On 05/20/2014 09:03 AM, Tucker DiNapoli wrote: >> Hello everyone, >> My name is Tucker DiNapoli and I'm going to be working on libvirt this >> summer for the google summer of code. My project is to implement job >> control in the storage driver. In order to do that I am first going to >> create a unified api for job control in libvirt (as it is currently >> implemented seperately in both the qemu and libxl drivers). I'm currently >> working on the api for job control and I have some ideas that I'll post on >> the mailing list soon. If anyone has any ideas or requests re job control >> I'd be happy to hear them. I'm on the #virt irc channel as tuckerD, I look >> forward to working on libvrt this summer. > > Welcome! I definitely have some ideas on the matter: > > https://www.redhat.com/archives/libvir-list/2014-February/msg01128.html > > and in notes I've taken from off-list communications: > >>> For that matter, we need to overhaul the job support. Right now, >>> virDomainBlockJob* assumes you can only have one outstanding job at a >>> time, which interferes with the notion of running fleecing from two >>> points in time. Similarly, there can only be one virDomainGetJobInfo >>> active. What we really need is a notion of a job id, a way to query the >>> job id most recently created by any existing job-related API, and a way >>> to query job status based on id rather than only the most-recently >>> started job. > > I'd _really_ like to see a common notion of a 'job id' that EVERY job > (whether domain-level, like migration; or block-level, like > commit/pull/rebase; or storage-level, like your new proposed storage > jobs) shares a common job namespace. The job id is a positive integer. > Existing APIs will have to be retrofitted into the new job id notion; > any action that starts a long-running job that currently returns 0 on > success could be changed to return a positive job id; or we may need a > new API that queries the notion of the 'current job' (the job most > recently started) or even to set the 'current job' to a different job > id. We'll need new API for querying a job by id, and to be most > portable, we should do job reporting via virTypedParameter > (virDomainGetJobInfo and virDomainGetBlockJobInfo are hardcoded into > returning a struct, so they are non-extensible; virDomainGetJobStats > almost did it right, except that the user has to call it twice, once to > learn how large to allocate, and again to pass in pre-allocated memory - > the ideal API would allocate the memory on a single call). > > The fact that many jobs are exclusive (only one domain job at a time, > and only one block job per disk) is currently inherent in the design of > not having a job id; but we eventually WANT to allow parallel jobs. So > I'm looking forward to reviewing your proposals. > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/14 18:21, Eric Blake wrote: > On 05/20/2014 10:16 AM, Eric Blake wrote: >> So I'm not quite sure where you are heading with this patch - is the >> idea that we WANT to allow an external checkpoint with no disks, and >> this is fixing a bug that gave an unknown error, or is this a case where >> we want to forbid diskless external checkpoints, and you are just >> improving the error message? > > More concretely, I _want_ to allow a memory-only external snapshot Well, this patch exactly does that. That's why there's no "after" error message. The commit message isn't probably clear enough. How about changing one of the sentences to: Rework early failure checking and set the error code to success before iterating the array of disks so that we return success if no disks are snapshotted. > (particularly of a diskless VM). If this patch is fixing a bogus error > message into allowing that case, then please touch up the commit message > to make that a bit more clear, and I'm fine with including the patch (my > initial read was that this patch is just improving error message quality > but still leaving it an error, but on reflection it looks like I > mis-read the intent because of confusion about the commit message). > Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/2014 10:55 AM, Peter Krempa wrote: > How about changing one of the sentences to: > > Rework early failure checking and set the error code to success before > iterating the array of disks so that we return success if no disks are > snapshotted. Works for me. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: snapshot: Fix return value of external checkpoint with no disks
On 05/20/14 19:23, Eric Blake wrote: > Rework early failure checking and set the error code to success before >> iterating the array of disks so that we return success if no disks are >> snapshotted. Thanks; Series pushed with suggested fixes. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Bug: iohelper drops I/O error messages
During a recent managed save operation I received the following error message: error: operation failed: domain save job: unexpectedly failed. It turns out that I had run out of disk space. After a brief investigation I discovered that libvirt_iohelper is exec'ed and is used to handle all I/O during a (Qemu) managed save operation. While iohelper appears to be set up to log error conditions when they occur, for some reason the logging is getting lost. I'm hoping someone can help figure out why these errors are getting lost. It would be nice to present a useful error message to the user when a managed save fails because of an I/O error. -- -- Jason J. Herne (jjhe...@linux.vnet.ibm.com) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] [PATCH] Remove #if 0's from libvirt-gobject-domain-snapshot
The code seems to be fine. --- I'd like to use this code (in later patches). If anything is wrong with the current implementation, I'd like to know so I can fix it. libvirt-gobject/libvirt-gobject-domain-snapshot.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c b/libvirt-gobject/libvirt-gobject-domain-snapshot.c index d4e9b97..ab23342 100644 --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c @@ -144,9 +144,7 @@ typedef struct virDomainSnapshot GVirDomainSnapshotHandle; static GVirDomainSnapshotHandle* gvir_domain_snapshot_handle_copy(GVirDomainSnapshotHandle *src) { -#if 0 virDomainSnapshotRef((virDomainSnapshotPtr)src); -#endif return src; } @@ -162,7 +160,7 @@ G_DEFINE_BOXED_TYPE(GVirDomainSnapshotHandle, gvir_domain_snapshot_handle, const gchar *gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot) { g_return_val_if_fail(GVIR_IS_DOMAIN_SNAPSHOT(snapshot), NULL); -#if 0 + GVirDomainSnapshotPrivate *priv = snapshot->priv; const char *name; @@ -172,12 +170,6 @@ const gchar *gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot) } return name; -#else -if (snapshot) -return NULL; -#endif - -g_return_val_if_reached(NULL); } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: avoid dlsym mocking on mingw
I got a build failure when cross-compiling to mingw with the mingw64-dbus package installed: CC virmockdbus_la-virmockdbus.lo ../../tests/virmockdbus.c:29:6: error: 'dbus_connection_set_change_sigpipe' redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes] VIR_MOCK_STUB_VOID_ARGS(dbus_connection_set_change_sigpipe, ^ ../../tests/virmockdbus.c:33:18: error: 'dbus_bus_get' redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes] VIR_MOCK_STUB_RET_ARGS(dbus_bus_get, ... Well duh - mingw lacks dlopen and friends, even if it can support dbus. A similar failure occured in virsystemdtest.c; but in that file, we know that systemd is a Linux-only concept. * tests/virmockdbus.c: Cripple on mingw. * tests/virsystemdtest.c: Cripple on non-Linux. Signed-off-by: Eric Blake --- Pushing under the build-breaker rule tests/virmockdbus.c| 6 +++--- tests/virsystemdtest.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/virmockdbus.c b/tests/virmockdbus.c index 8a01d9d..49db98c 100644 --- a/tests/virmockdbus.c +++ b/tests/virmockdbus.c @@ -1,7 +1,7 @@ /* * virmockdbus.c: mocking of dbus message send/reply * - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -22,7 +22,7 @@ #include -#ifdef WITH_DBUS +#if defined(WITH_DBUS) && !defined(WIN32) # include "virmock.h" # include @@ -61,4 +61,4 @@ VIR_MOCK_LINK_RET_ARGS(dbus_connection_send_with_reply_and_block, int, timeout_milliseconds, DBusError *, error) -#endif /* WITH_DBUS */ +#endif /* WITH_DBUS && !WIN32 */ diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 0fcd4e8..8f7b47e 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -22,7 +22,7 @@ #include "testutils.h" -#ifdef WITH_DBUS +#if defined(WITH_DBUS) && defined(__linux__) # include # include @@ -477,7 +477,7 @@ mymain(void) VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmockdbus.so") -#else /* ! WITH_DBUS */ +#else /* ! (WITH_DBUS && __linux__) */ int main(void) { -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V7] libxl: add migration support
Jim Fehlig wrote: > This patch adds initial migration support to the libxl driver, > using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration > functions. > Any comments on this version of the patch? Regards, Jim > Signed-off-by: Jim Fehlig > --- > > V6 here > https://www.redhat.com/archives/libvir-list/2014-May/msg00017.html > > In V7: > There were no comments on V6, but during testing I noticed that > 'virsh migrate --suspend ...' was not being honored. Fixed that > in this version so that the domain is paused on the destination > once migration completes. > > po/POTFILES.in | 1 + > src/Makefile.am | 3 +- > src/libxl/libxl_conf.h | 6 + > src/libxl/libxl_domain.h| 1 + > src/libxl/libxl_driver.c| 235 ++ > src/libxl/libxl_migration.c | 581 > > src/libxl/libxl_migration.h | 79 ++ > 7 files changed, 905 insertions(+), 1 deletion(-) > > diff --git a/po/POTFILES.in b/po/POTFILES.in > index e35eb82..a72dc1e 100644 > --- a/po/POTFILES.in > +++ b/po/POTFILES.in > @@ -73,6 +73,7 @@ src/lxc/lxc_process.c > src/libxl/libxl_domain.c > src/libxl/libxl_driver.c > src/libxl/libxl_conf.c > +src/libxl/libxl_migration.c > src/network/bridge_driver.c > src/network/bridge_driver_linux.c > src/node_device/node_device_driver.c > diff --git a/src/Makefile.am b/src/Makefile.am > index e9dc9e0..0dbda7f 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -706,7 +706,8 @@ XENAPI_DRIVER_SOURCES = > \ > LIBXL_DRIVER_SOURCES = \ > libxl/libxl_conf.c libxl/libxl_conf.h \ > libxl/libxl_domain.c libxl/libxl_domain.h \ > - libxl/libxl_driver.c libxl/libxl_driver.h > + libxl/libxl_driver.c libxl/libxl_driver.h \ > + libxl/libxl_migration.c libxl/libxl_migration.h > > UML_DRIVER_SOURCES = \ > uml/uml_conf.c uml/uml_conf.h \ > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h > index 24e1720..b798567 100644 > --- a/src/libxl/libxl_conf.h > +++ b/src/libxl/libxl_conf.h > @@ -43,6 +43,9 @@ > # define LIBXL_VNC_PORT_MIN 5900 > # define LIBXL_VNC_PORT_MAX 65535 > > +# define LIBXL_MIGRATION_PORT_MIN 49152 > +# define LIBXL_MIGRATION_PORT_MAX 49216 > + > # define LIBXL_CONFIG_DIR SYSCONFDIR "/libvirt/libxl" > # define LIBXL_AUTOSTART_DIR LIBXL_CONFIG_DIR "/autostart" > # define LIBXL_STATE_DIR LOCALSTATEDIR "/run/libvirt/libxl" > @@ -115,6 +118,9 @@ struct _libxlDriverPrivate { > /* Immutable pointer, self-locking APIs */ > virPortAllocatorPtr reservedVNCPorts; > > +/* Immutable pointer, self-locking APIs */ > +virPortAllocatorPtr migrationPorts; > + > /* Immutable pointer, lockless APIs*/ > virSysinfoDefPtr hostsysinfo; > }; > diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h > index 979ce2a..9d48049 100644 > --- a/src/libxl/libxl_domain.h > +++ b/src/libxl/libxl_domain.h > @@ -69,6 +69,7 @@ struct _libxlDomainObjPrivate { > virChrdevsPtr devs; > libxl_evgen_domain_death *deathW; > libxlDriverPrivatePtr driver; > +unsigned short migrationPort; > > struct libxlDomainJobObj job; > }; > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index df7d510..35467ac 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -45,6 +45,7 @@ > #include "libxl_domain.h" > #include "libxl_driver.h" > #include "libxl_conf.h" > +#include "libxl_migration.h" > #include "xen_xm.h" > #include "xen_sxpr.h" > #include "virtypedparam.h" > @@ -209,6 +210,7 @@ libxlStateCleanup(void) > virObjectUnref(libxl_driver->xmlopt); > virObjectUnref(libxl_driver->domains); > virObjectUnref(libxl_driver->reservedVNCPorts); > +virObjectUnref(libxl_driver->migrationPorts); > > virObjectEventStateFree(libxl_driver->domainEventState); > virSysinfoDefFree(libxl_driver->hostsysinfo); > @@ -301,6 +303,13 @@ libxlStateInitialize(bool privileged, >LIBXL_VNC_PORT_MAX))) > goto error; > > +/* Allocate bitmap for migration port reservation */ > +if (!(libxl_driver->migrationPorts = > + virPortAllocatorNew(_("migration"), > + LIBXL_MIGRATION_PORT_MIN, > + LIBXL_MIGRATION_PORT_MAX))) > +goto error; > + > if (!(libxl_driver->domains = virDomainObjListNew())) > goto error; > > @@ -4155,6 +4164,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int > feature) > > switch (feature) { > case VIR_DRV_FEATURE_TYPED_PARAM_STRING: > +case VIR_DRV_FEATURE_MIGRATION_PARAMS: > return 1; > default: > return 0; > @@ -4333,6 +4343,226 @@ libxlNodeDeviceReset(virNodeDeviceP
Re: [libvirt] RFC: Any interest in a weekly(?) dev community meeting ?
Daniel P. Berrange wrote: > On Fri, May 16, 2014 at 01:53:50PM -0400, Daniel P. Berrange wrote: > >> Hi Libvirt team, >> >> A number of opensource projects have weekly meetings between their community >> of contributors to facilitate their day-to-day working and particularly >> to resolve roadblocks that people are having. >> >> I feel that libvirt is large enough, with contributors from many different >> organizations, that a meeting could be beneficial to our operation. This >> could serve a number of purposes >> >> - Remind us of patches that have been posted and accidentally forgotten >>by reviewers >> >> - Resolve hard debates that are not making adequate progress on the >>mailing list(s) >> >> - Track progress of major ongoing pieces of work with many collaborators >> >> - Forum for those new to the community to introduce themselves & their >>ideas before starting work >> >> - Discuss release critical bugs during freeze periods >> >> - Place for downstream users of libvirt (eg openstack/ovirt/etc) to >>interact with libvirt team. >> >> - Place for projects we use (eg KVM, Xen) to interact with libvirt >>team. >> >> I like to keep the overhead of this low, so I'd suggest we try todo it >> on IRC, since that has been fairly effective for OpenStack teams. >> >> If people think this is worth while I'd suggest an arbitrary time of >> 1500 UTC using the #virt-meeting IRC channel on irc.oftc.net, to last an >> absolute max of 1 hour. Currently this time point works out as >> > > Opps, missd the day - I was meaning to suggest > > *1500 UTC on thursdays* > Thanks Daniel. I think this is a great idea, and unlike the similar proposal you made for a nova libvirt sub-team meeting, I'm available during this time slot. Well, with the exception of this week. Were you planning the first meeting this Thursday, as in May 22nd? Regards, Jim > >> 08:00 San Francisco >> 11:00 Boston >> 15:00 UTC >> 16:00 London >> 17:00 Berlin >> 20:30 Mumbai >> 23:00 Bejing >> 24:00 Tokyo >> >> >> http://www.timeanddate.com/worldclock/fixedtime.html?hour=15&min=00&sec=0p1=0 >> >> > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Any interest in a weekly(?) dev community meeting ?
On 05/16/2014 11:53 AM, Daniel P. Berrange wrote: > > If people think this is worth while I'd suggest an arbitrary time of > 1500 UTC using the #virt-meeting IRC channel on irc.oftc.net, to last an > absolute max of 1 hour. Currently this time point works out as Is this tied to true UTC (no daylight savings, so everyone living in a timezone with daylight savings has to adjust their local start time twice a year) or to a particular timezone (where some people always have the same local time, while other unlucky folks might have to adjust their own start time as many as 4 times a year because of differences between daylight savings cutoff points)? I'm definitely in favor of the idea. However, in my personal situation, starting an hour earlier (1400 UTC) would fit a bit better for Thursday meetings. That's because I generally try to attend Austin Group teleconferences [the standards body in charge of POSIX] which are also Thursdays currently at 1500 UTC (but tied to US daylight savings rules); and my close involvement with POSIX has come in handy for libvirt more than once. I've got more flexibility if we pick a different day of the week. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virsh reconnect vs. keepalive issue
I'm trying to debug a new virsh command, and had two terminals up, one running './run gdb tools/virsh' (I originally ran virsh directly, but re-ran under gdb to get a trace) and the other running './run gdb daemon/libvirtd'. Because I forgot to use the -k0 argument to disable keepalives, and took too long while at a breakpoint on the libvirtd terminal, my virsh session tried to report a keepalive failure: [New Thread 0x7fffe7b8c700 (LWP 7143)] 2014-05-20 23:40:22.337+: 7138: info : libvirt version: 1.2.5 2014-05-20 23:40:22.337+: 7138: warning : virKeepAliveTimerInternal:143 : No response from client 0x5586a520 after 6 keepalive messages in 35 seconds 2014-05-20 23:40:22.337+: 7143: warning : virKeepAliveTimerInternal:143 : No response from client 0x5586a520 after 6 keepalive messages in 35 seconds error: internal error: received hangup / error event on socket but then it just hangs there. A backtrace shows that it is hanging trying to reconnect: (gdb) bt #0 0x739129dd in poll () at ../sysdeps/unix/syscall-template.S:81 #1 0x7798e3c1 in virNetClientIOEventLoop (client=0x5586ae10, thiscall=0x5586b090) at rpc/virnetclient.c:1514 #2 0x7798ecbf in virNetClientIO (client=0x5586ae10, thiscall=0x5586b090) at rpc/virnetclient.c:1786 #3 0x7798f5ab in virNetClientSendInternal (client=0x5586ae10, msg=0x5586ada0, expectReply=true, nonBlock=false) at rpc/virnetclient.c:1958 #4 0x7798f648 in virNetClientSendWithReply (client=0x5586ae10, msg=0x5586ada0) at rpc/virnetclient.c:1986 #5 0x77990580 in virNetClientProgramCall (prog=0x5586afd0, client=0x5586ae10, serial=0, proc=66, noutfds=0, outfds=0x0, ninfds=0x0, infds=0x0, args_filter=0x7394f270 <__GI_xdr_void>, args=0x0, ret_filter=0x7798152c , ret=0x7fffd6f0) at rpc/virnetclientprogram.c:329 #6 0x77975d4c in callFull (conn=0x55869fb0, priv=0x5586ab30, flags=0, fdin=0x0, fdinlen=0, fdout=0x0, fdoutlen=0x0, proc_nr=66, args_filter=0x7394f270 <__GI_xdr_void>, args=0x0, ret_filter=0x7798152c , ret=0x7fffd6f0 "") at remote/remote_driver.c:6544 #7 0x77975e25 in call (conn=0x55869fb0, priv=0x5586ab30, flags=0, proc_nr=66, args_filter=0x7394f270 <__GI_xdr_void>, args=0x0, ret_filter=0x7798152c , ---Type to continue, or q to quit--- ret=0x7fffd6f0 "") at remote/remote_driver.c:6566 #8 0x7795f7f1 in remoteAuthenticate (conn=0x55869fb0, priv=0x5586ab30, auth=0x77d71520 , authtype=0x0) at remote/remote_driver.c:3939 #9 0x77957f0a in doRemoteOpen (conn=0x55869fb0, priv=0x5586ab30, auth=0x77d71520 , flags=0) at remote/remote_driver.c:973 #10 0x7795890b in remoteConnectOpen (conn=0x55869fb0, auth=0x77d71520 , flags=0) at remote/remote_driver.c:1190 #11 0x778fc5e5 in do_open (name=0x0, auth=0x77d71520 , flags=0) at libvirt.c:1147 #12 0x778fd093 in virConnectOpenAuth (name=0x0, auth=0x77d71520 , flags=0) at libvirt.c:1390 #13 0x55575a67 in vshConnect (ctl=0x7fffdbc0, uri=0x0, readonly=false) at virsh.c:337 #14 0x55575c23 in vshReconnect (ctl=0x7fffdbc0) at virsh.c:383 #15 0x5557943a in vshCommandRun (ctl=0x7fffdbc0, cmd=0x557f4900) at virsh.c:1853 #16 0x5557d8de in main (argc=5, argv=0x7fffdda8) at virsh.c:3617 It seems odd that virsh is hanging rather than gracefully exit, because it is trying too hard to reconnect in spite of being forcefully disconnected due to a keepalive timeout. But I'm not sure what, if anything, we could tweak to make this failure mode not require a Ctrl-C on my end to get rid of the failed virsh. It also reiterates the idea that we really ought to allow for a user-specified timeout to virConnect, rather than blocking forever. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
Hi Cole, The fact is that we need maintain several patches to make libvirt to work with qemu-system-ppc on ppce500 platform. 1) enable QEMU_CAPS_PCI_MULTIBUS 2) use legacy -serial option The first one is not needed any more since libvirt v1.2.4. This patch is trying to fix the second one. The test case qemuxml2argv-ppc-dtb.args was used to demonstrate -dtb option. Anyway it also used -serial option otherwise the domain could not work on ppce500 platform. I mentioned the test case qemuxml2argv-pseries-basic.args just for the usage of '-chardev spapr-vty' on pseries. We don't want to do any change to the use cases on pseries. Best Regards, Olivia > -Original Message- > From: Cole Robinson [mailto:crobi...@redhat.com] > Sent: Tuesday, May 20, 2014 8:51 PM > To: Yin Olivia-R63875; libvir-list@redhat.com; zhlci...@linux.vnet.ibm.com > Subject: Re: [PATCH] qemu: Fix specifying char devs for PPC > > On 05/20/2014 01:25 AM, hong-hua@freescale.com wrote: > > Hi Cole, > > > > Thanks for the comments. > > Exactly there were already test cases for both pseries and ppce500 > machines. > > For example, > > 1) qemuxml2argv-pseries-basic.args: '-chardev spapr-vty' for > > pseries. > > 2) qemuxml2argv-ppc-dtb.args: '-serial pty' for ppce500. > > > > If there's already test cases, then what does the patch actually change? > What I'm suggesting is to add a test case that would fail before this patch, > and succeed afterwards, to demonstrate what is actually changing. > > - Cole > > > > > > > >> -Original Message- > >> From: Cole Robinson [mailto:crobi...@redhat.com] > >> Sent: Monday, May 19, 2014 9:40 PM > >> To: Yin Olivia-R63875; libvir-list@redhat.com; > >> zhlci...@linux.vnet.ibm.com > >> Subject: Re: [PATCH] qemu: Fix specifying char devs for PPC > >> > >> On 05/19/2014 03:41 AM, hong-hua@freescale.com wrote: > >>> Hi Cole, > >>> > >>> This is a patch similar with your previous patch to fix for ARM. > >>> Do you have any comments on it? > >>> > >>> Cindy, > >>> Since you are the main contributor to QEMU driver on PPC, I'll also > >> appreciate your comments. > >>> > >>> Best Regards, > >>> Olivia > >>> > >> > >> Patch looks fine, but it should add a qemuxml2argv test case to > >> validate that it actually works. My original patch added test cases > >> later in the patch series, see > >> 54a77c6df3c483864463f602c4c6f435d50bd79e > >> > >> - Cole > >> > -Original Message- > From: Yin Olivia-R63875 > Sent: Friday, May 16, 2014 8:38 AM > To: Yin Olivia-R63875; libvir-list@redhat.com > Subject: RE: [PATCH] qemu: Fix specifying char devs for PPC > > Ping. > > This is a patch similar with ARM platforms. > http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc > 96 > 8171c5 > 84ec2edcfdcb8fadde > > Right now on ppce500, chardev is not supported for the serial > console. So it uses the the legacy -serial option. > > Best Regards, > Olivia > > > -Original Message- > > From: Olivia Yin [mailto:hong-hua@freescale.com] > > Sent: Wednesday, May 14, 2014 6:47 PM > > To: libvir-list@redhat.com > > Cc: Yin Olivia-R63875 > > Subject: [PATCH] qemu: Fix specifying char devs for PPC > > > > QEMU ppce500 board uses the old style -serial options. > > > > Other PPC boards don't give any way to explicitly wire in a > > -chardev except pseries which uses -device spapr-vty with -chardev. > > > > --- > > src/qemu/qemu_capabilities.c | 10 +++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/src/qemu/qemu_capabilities.c > > b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644 > > --- a/src/qemu/qemu_capabilities.c > > +++ b/src/qemu/qemu_capabilities.c > > @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr > def, > > !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) > > return false; > > > > -if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != > > VIR_ARCH_AARCH64)) > > +if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != > > VIR_ARCH_AARCH64) > > +&& (def->os.arch != VIR_ARCH_PPC) && (def->os.arch != > > +VIR_ARCH_PPC64)) > > return true; > > > > /* This may not be true for all ARM machine types, but at least > > * the only supported non-virtio serial devices of vexpress > > and versatile > > - * don't have the -chardev property wired up. */ > > + * don't have the -chardev property wired up. > > + * For PPC machines, only pserial need -device spapr-vty with > > + -chardev */ > > return (chr->info.type == > > VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO > > || > > (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE > && > > - ch
Re: [libvirt] [PATCH v6] migration: add support for migrateHost configuration
On Tue, 2014-05-20 at 13:47 +0200, Jiri Denemark wrote: > On Tue, May 20, 2014 at 18:26:04 +0800, Chen Fan wrote: > > For now, we set the migration URI via command line '--migrate_uri' or > > construct the URI by looking up the dest host's hostname which could be > > solved by DNS automatically. > > > > But in cases the dest host have two or more NICs to reach, we may need to > > send the migration data over a specific NIC which is different from the > > automatically resloved one for some reason like performance, security, etc. > > thus we must explicitly specify the migrateuri in command line everytime, > > but it is too troublesome if there are many such hosts(and don't forget > > virt-manager). > > > > This patch adds a configuration file option on dest host to save the > > default value set which can be specified to a migration hostname or > > one of this host's addresses used for transferring data, thus user doesn't > > boring to specify it in command line everytime. > > > > Signed-off-by: Chen Fan > > --- > > src/qemu/libvirtd_qemu.aug | 1 + > > src/qemu/qemu.conf | 9 - > > src/qemu/qemu_conf.c | 1 + > > src/qemu/qemu_conf.h | 1 + > > src/qemu/qemu_migration.c | 21 + > > src/qemu/test_libvirtd_qemu.aug.in | 1 + > > 6 files changed, 29 insertions(+), 5 deletions(-) > > Oops, I already took v5, applied the changes I suggested and Daniel > ACKed and pushed the result. > > So this is already pushed now. Ok, Thanks. Chen > > Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
On 05/20/2014 07:25 AM, hong-hua@freescale.com wrote: > Hi Cole, > > Thanks for the comments. > Exactly there were already test cases for both pseries and ppce500 machines. > For example, > 1) qemuxml2argv-pseries-basic.args: '-chardev spapr-vty' for pseries. > 2) qemuxml2argv-ppc-dtb.args: '-serial pty' for ppce500. The dtb test is run as: DO_TEST("ppc-dtb", QEMU_CAPS_KVM, QEMU_CAPS_DTB); You need a test which is run with both QEMU_CAPS_DEVICE and QEMU_CAPS_CHARDEV capabilities to test the changes in virQEMUCapsSupportsChardev, otherwise it just returns false at the beginning. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
On 2014年05月19日 15:41, hong-hua@freescale.com wrote: Hi Cole, This is a patch similar with your previous patch to fix for ARM. Do you have any comments on it? Cindy, Since you are the main contributor to QEMU driver on PPC, I'll also appreciate your comments. Hi Hong-hua, It looks good for pseries. You may need to add a test case as Cole suggests. Thanks. --Li Best Regards, Olivia -Original Message- From: Yin Olivia-R63875 Sent: Friday, May 16, 2014 8:38 AM To: Yin Olivia-R63875; libvir-list@redhat.com Subject: RE: [PATCH] qemu: Fix specifying char devs for PPC Ping. This is a patch similar with ARM platforms. http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc968171c5 84ec2edcfdcb8fadde Right now on ppce500, chardev is not supported for the serial console. So it uses the the legacy -serial option. Best Regards, Olivia -Original Message- From: Olivia Yin [mailto:hong-hua@freescale.com] Sent: Wednesday, May 14, 2014 6:47 PM To: libvir-list@redhat.com Cc: Yin Olivia-R63875 Subject: [PATCH] qemu: Fix specifying char devs for PPC QEMU ppce500 board uses the old style -serial options. Other PPC boards don't give any way to explicitly wire in a -chardev except pseries which uses -device spapr-vty with -chardev. --- src/qemu/qemu_capabilities.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64)) +if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64) +&& (def->os.arch != VIR_ARCH_PPC) && (def->os.arch != +VIR_ARCH_PPC64)) return true; /* This may not be true for all ARM machine types, but at least * the only supported non-virtio serial devices of vexpress and versatile - * don't have the -chardev property wired up. */ + * don't have the -chardev property wired up. + * For PPC machines, only pserial need -device spapr-vty with + -chardev */ return (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); + chr->targetType == + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) || +(chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)); } -- 1.8.5 -- Li Zhang IBM China Linux Technology Centre -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
On 2014年05月21日 14:00, Ján Tomko wrote: On 05/20/2014 07:25 AM, hong-hua@freescale.com wrote: Hi Cole, Thanks for the comments. Exactly there were already test cases for both pseries and ppce500 machines. For example, 1) qemuxml2argv-pseries-basic.args: '-chardev spapr-vty' for pseries. 2) qemuxml2argv-ppc-dtb.args: '-serial pty' for ppce500. The dtb test is run as: DO_TEST("ppc-dtb", QEMU_CAPS_KVM, QEMU_CAPS_DTB); You need a test which is run with both QEMU_CAPS_DEVICE and QEMU_CAPS_CHARDEV capabilities to test the changes in virQEMUCapsSupportsChardev, otherwise it just returns false at the beginning. Hi Hong-Hua, Jan is right. That case doesn't test the code which is changed. Because QEMU_CAPS_DEVICE and QEMU_CAPS_CHARDEV is not set, it won't run into your code. You can add these two capabilities to this case to verify your code. :) Thanks. Li Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list