Re: [libvirt] Downloading and wiping assumes volume is a device or file
Hi, On 02/09/2012 09:27 PM, Eric Blake wrote: On 02/09/2012 01:04 PM, Wido den Hollander wrote: Hi, I'm still working on the RBD (RADOS / Ceph) storage driver for libvirt and I noticed the virStorageVolDownload and virStorageVolWipe methods. I assumed those would be passed on to the storage backend, but it doesn't. In the storageDriver the method storageVolumeDownload simply opens a file descriptor and reads the device. Until now libvirt only had support for storage drivers who presented regular files or block devices, but RBD doesn't. (Well, RBD could, but I'm currently going for Qemu-RBD). In the future we might see more storage drivers in libvirt for a project like Sheepdog as well. Sheepdog and RBD both have drivers in Qemu. What would the way be to approach this? Should the download, upload and wipe methods be moved to the storage backends? There could also be an exception? If virStoragePoolType matches VIR_STORAGE_POOL_RBD or VIR_STORAGE_POOL_SHEEPDOG the storage backend could be invoked instead of opening the file descriptor? Any thoughts on this? I was just checking out this code again and I saw that both RBD and Sheepdog prefix their device paths RBD: rbd:pool/image Sheepdog: sheepdog:image In the storageDriver a check could be made. If 'path' is a file we use the regular methods. If it's a prefix, we send this down to the storage backend for further handling. In short-term this doesn't brake any exisiting code and applications, but does allow these functions to work with new storage backends. Wido For at least virStorageVolDownload, I can see two useful behaviors, and think we need to introduce a flag to specify which behavior: virStorageVolDownload(, 0) = verbatim copy of the raw volume, including metadata, as seen by the host; does not require any decryption keys to download, but may require secrets for accessing remote Sheepdog serves - great for cloning volumes virStorageVolDownload(, CONTENTS) = copy the contents of the volume as they would be seen in the guest; for any encrypted storage formats, this requires the right virSecret object to access the contents - great for converting arbitrary volumes into a raw storage format Conversely, virStorageVolUpload(, 0) installs a raw volume, and virStroageVolUpload(, CONTENTS) takes raw data and injects it into the contents that the guest will see (which may involve compression or encryption). Even on just local file systems, this would be the difference between 'cp qcow2.img1 qcow2.img2' (copy the entire metadata, without knowing the contents as seen by the guest) and 'qemu-img conviert -f qcow2 -O qcow2 qcow2.img1 qcow2.img2' (copy the entire guest contents, but by creating new qcow2 metadata, which may result in a smaller img2 file as abandoned sectors in img1 are elided). Very much worth doing! I also think that virStorageVolWipe should have a raw vs. contents, as in: virStorageVolWipe(, 0) = wipe the entire volume, including any storage metadata; a qcow2 file with 10M in use, 30M capacity, and occupying 20M on disk gets wiped into a raw file with 20M in use, capacity, and allocation virStorageVolWipe(, CONTENTS) = ensure that the guest sees an empty volume, but preserve host metadata; a qcow2 file with 10M in use, 30M capacity, and occupying 20M on disk gets converted to a qcow2 with 0 bytes in use, 30M capacity; and we probably need a second flag to control whether disk usage is altered (that is, whether the image is thin or pre-allocated on disk) It certainly sounds like we should be wiring more of these decisions through to the backends, rather than attempting to do them all from the main storage driver. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: combine three bools in virNetDevTapCreateInBridgePort into one flags
On 03/01/2012 03:59 PM, Eric Blake wrote: On 03/01/2012 01:48 PM, Laine Stump wrote: With an additional new bool added to determine whether or not to discourage the use of the supplied MAC address by the bridge itself, virNetDevTapCreateInBridgePort had three booleans (well, 2 bools and an int used as a bool) in the arg list, which made it increasingly difficult to follow what was going on. This patch combines those three into a single flags arg, which not only shortens the arg list, but makes it more self-documenting. --- Does this make more sense as a PATCH 2/1 to be associated with the first patch in this thread: http://www.redhat.com/archives/libvir-list/2012-February/msg00760.html or should I squash them both together? (I'm leaning towards two separate patches, but could be convinced either way) I'm fine with two patches. I haven't reviewed the earlier post, but for this commit, you have my: ACK. Thanks. I just pushed both patches. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Use the same MAC address that is defined in domain XML for attached-mac field.
On 02/16/2012 06:49 PM, Ansis Atteka wrote: Currently libvirt sets the attached-mac to altered MAC address that has first byte set to FE. This patch will change that behavior by using the original (unaltered) MAC address from the domain XML configuration file. Okay, after much discussion, and pairing this patch with another that combines the now-three bools in the arglist of virNetDevTapCreateInBridgePort into a single flags to make it easier to understand: ACK. I wrote a more descriptive commit notice for this patch (explaining how the whole 0xFE thing works and why), and pushed it together with the aforementioned 3 x bool -- 1 flags patch. Thanks for your patience :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch]: spice agent-mouse support
On Fri, Mar 02, 2012 at 09:06:20AM +0800, Zhou Peng wrote: On Thu, Mar 1, 2012 at 5:32 PM, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Mar 01, 2012 at 11:54:30AM +0800, Zhou Peng wrote: Signed-off-by: Zhou Peng zhoup...@nfs.iscas.ac.cn spice agent-mouse support diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml index 6505b55..266a4ed 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml @@ -23,7 +23,7 @@ controller type='virtio-serial' index='1' address type='pci' domain='0x' bus='0x00' slot='0x0a' function='0x0'/ /controller - graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' + graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' agentmouse='off' channel name='main' mode='secure'/ /graphics channel type='spicevmc' Rather than adding an attribute 'agentmouse' I think it'd be preferrable to use a sub-element: agent mouse='on|off'/ that way if we have more configuration related to the agent we have a nice place to put it I take note of the implemented clipboard sub-element.. spice clipboard use agent too to implement copypaste like agentmouse. I realize your idea to separate is great in the long run and agree with you to use another sub-emement to describe, But I'm sorry I still don't agree with you to use agent mouse='on|off'/ How about this way pls: graphics type='spice' mouse mode='client|server' Yes this looks fine to me. ... clipboard copypaste='yes|no'/ /graphics Refering to qemu-spice's implement: There are two mouse modes at the moment that is SPICE_MOUSE_MODE_SERVER and SPICE_MOUSE_MODE_CLIENT Currently 'agent-mouse=on' equal to 'SPICE_MOUSE_MODE_CLIENT' 'agent-mouse=off' equal to 'SPICE_MOUSE_MODE_SERVER' Yes, your idea makes sense in this context 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] build: prohibit cross-inclusion
On Thu, Mar 01, 2012 at 05:40:19PM -0700, Eric Blake wrote:
Make it easier to detect invalid cross-directory includes, by
adding a syntax check. The check is designed to be extensible:
the default case lists only the non-driver directories, and
specific directories can list a different set (for example,
util/ can only use itself, network/ can only use itself, util/,
or conf/).
* .gnulib: Update to latest, for syntax check improvment.
* cfg.mk (sc_prohibit_cross_inclusion): New check.
(sc_prohibit_strncmp, sc_libvirt_unmarked_diagnostics): Simplify.
---
* .gnulib b856fad...44de969 (31):
maint.mk: avoid spurious failure of _sc_search_regexp-using tests
maint.mk: add per-line exclusions to prohibitions
Tests for module 'expl-ieee'.
New module 'expl-ieee'.
Tests for module 'exp-ieee'.
New module 'exp-ieee'.
Tests for module 'expf-ieee'.
New module 'expf-ieee'.
cbrtl-ieee: Work around test failure on IRIX 6.5.
Tests for module 'cbrtl-ieee'.
New module 'cbrtl-ieee'.
Tests for module 'cbrt-ieee'.
New module 'cbrt-ieee'.
Tests for module 'cbrtf-ieee'.
New module 'cbrtf-ieee'.
cbrtf: Work around bug in IRIX 6.5 system function.
Tests for module 'cbrtl'.
New module 'cbrtl'.
Tests for module 'cbrtf'.
New module 'cbrtf'.
cbrt: Provide replacement on MSVC and Minix.
hypotl-ieee: Work around test failure on OSF/1 and native Windows.
hypotf-ieee: Work around test failure on OSF/1 and native Windows.
hypot-ieee: Work around test failure on OSF/1 and native Windows.
Tests for module 'hypotl-ieee'.
New module 'hypotl-ieee'.
Tests for module 'hypot-ieee'.
New module 'hypot-ieee'.
Tests for module 'hypotf-ieee'.
New module 'hypotf-ieee'.
Remove unused variables.
.gnulib |2 +-
cfg.mk | 38 --
2 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/.gnulib b/.gnulib
index b856fad..44de969 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit b856fadc1c8dcb53e7efcbb2d0ae7edc022fdb6a
+Subproject commit 44de969cd62abbfe3e7cc7641a8dea7673fd2d6d
diff --git a/cfg.mk b/cfg.mk
index ac6c527..95994df 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -348,11 +348,10 @@ sc_prohibit_access_xok:
# Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0.
snp_ = strncmp *\(.+\)
sc_prohibit_strncmp:
- @grep -nE '! *strncmp *\(|\$(snp_) *[!=]=|[!=]= *$(snp_)' \
- $$($(VC_LIST_EXCEPT)) \
- | grep -vE ':# *define STR(N?EQLEN|PREFIX)\(' \
- { echo '$(ME): use STREQLEN or STRPREFIX instead of str''ncmp' \
- 12; exit 1; } || :
+ @prohibit='! *strncmp *\(|\$(snp_) *[!=]=|[!=]= *$(snp_)' \
+ exclude=':# *define STR(N?EQLEN|PREFIX)\(' \
+ halt='$(ME): use STREQLEN or STRPREFIX instead of str''ncmp'\
+ $(_sc_search_regexp)
# Use virAsprintf rather than as'printf since *strp is undefined on error.
sc_prohibit_asprintf:
@@ -569,11 +568,10 @@ func_re := ($(func_or))
#_(...:
#%s, _(no storage vol w...
sc_libvirt_unmarked_diagnostics:
- @grep -nE \
- '\$(func_re) *\([^]*[^]*[a-z]{3}' $$($(VC_LIST_EXCEPT)) \
- | grep -v '_''('\
- { echo '$(ME): found unmarked diagnostic(s)' 12;\
- exit 1; } || :
+ @prohibit='\$(func_re) *\([^]*[^]*[a-z]{3}' \
+ exclude='_\(' \
+ halt='$(ME): found unmarked diagnostic(s)' \
+ $(_sc_search_regexp)
@{ grep -nE '\$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
grep -A1 -nE '\$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
| sed 's/_([^][^]*//;s/[ ]%s//' \
@@ -624,6 +622,26 @@ sc_prohibit_gettext_markup:
halt='do not mark these strings for translation'\
$(_sc_search_regexp)
+# Our code is divided into modular subdirectories for a reason, and
+# lower-level code must not include higher-level headers.
+cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.))
+cross_dirs_re=($(subst / ,/|,$(cross_dirs)))
+sc_prohibit_cross_inclusion:
+ @for dir in $(cross_dirs); do \
+ case $$dir in \
+ util/) safe=util;;\
+ cpu/ | locking/ | network/ | rpc/ | security/) \
+ safe=($$dir|util|conf);;\
+ xenapi/ | xenxs/ ) safe=($$dir|util|conf|xen);; \
+ *) safe=($$dir|util|conf|cpu|network|locking|rpc|security);; \
+ esac;
Re: [libvirt] [Patch]: spice agent-mouse support
On Fri, Mar 2, 2012 at 5:58 PM, Daniel P. Berrange berra...@redhat.com wrote: On Fri, Mar 02, 2012 at 09:06:20AM +0800, Zhou Peng wrote: On Thu, Mar 1, 2012 at 5:32 PM, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Mar 01, 2012 at 11:54:30AM +0800, Zhou Peng wrote: Signed-off-by: Zhou Peng zhoup...@nfs.iscas.ac.cn spice agent-mouse support diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml index 6505b55..266a4ed 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml @@ -23,7 +23,7 @@ controller type='virtio-serial' index='1' address type='pci' domain='0x' bus='0x00' slot='0x0a' function='0x0'/ /controller - graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' + graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' agentmouse='off' channel name='main' mode='secure'/ /graphics channel type='spicevmc' Rather than adding an attribute 'agentmouse' I think it'd be preferrable to use a sub-element: agent mouse='on|off'/ that way if we have more configuration related to the agent we have a nice place to put it I take note of the implemented clipboard sub-element.. spice clipboard use agent too to implement copypaste like agentmouse. I realize your idea to separate is great in the long run and agree with you to use another sub-emement to describe, But I'm sorry I still don't agree with you to use agent mouse='on|off'/ How about this way pls: graphics type='spice' mouse mode='client|server' Yes this looks fine to me. OK, I will resend a new version patch after this weekend. Thanks Daniel. -- Zhou Peng -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] build: use correct type for pid and similar types
On 02/11/2012 12:55 AM, Eric Blake wrote:
No thanks to 64-bit windows, with 64-bit pid_t, we have to avoid
constructs like 'int pid'. Our API in libvirt-qemu cannot be
changed without breaking ABI; but then again, libvirt-qemu can
only be used on systems that support UNIX sockets, which rules
out Windows (even if qemu could be compiled there) - so for all
points on the call chain that interact with this API decision,
we require a different variable name to make it clear that we
audited the use for safety.
Adding a syntax-check rule only solves half the battle; anywhere
that uses printf on a pid_t still needs to be converted, but that
will be a separate patch.
Sorry for remembering really late to review your patch.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1b92aa4..2e63193 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7925,7 +7926,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
pidfile, monConfig, monJSON)))
goto cleanup;
-if (virAsprintf(exepath, /proc/%u/exe, pid) 0) {
+if (virAsprintf(exepath, /proc/%u/exe, (int) pid) 0) {
I'd use /proc/%lld/exe and cast pid to (long long). Or at least change
%u to %d for the (int) typecast. I agree that linux does not use
long pids now, but it's nicer when the code is uniform and you used the
long long variant later on and in the 2/2 patch of this series.
virReportOOMError();
goto cleanup;
}
@@ -7933,7 +7934,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
if (virFileResolveLink(exepath,emulator) 0) {
virReportSystemError(errno,
_(Unable to resolve %s for pid %u),
- exepath, pid);
+ exepath, (int) pid);
Same as above.
goto cleanup;
}
VIR_FREE(def-emulator);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 160cb37..abe73f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1065,10 +1065,12 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int
*lastCpu, long *vm_rss,
int cpu;
int ret;
+/* In general, we cannot assume pid_t fits in int; but /proc parsing
+ * is specific to Linux where int works fine. */
The format string is correct here
if (tid)
-ret = virAsprintf(proc, /proc/%d/task/%d/stat, pid, tid);
+ret = virAsprintf(proc, /proc/%d/task/%d/stat, (int) pid, tid);
else
-ret = virAsprintf(proc, /proc/%d/stat, pid);
+ret = virAsprintf(proc, /proc/%d/stat, (int) pid);
if (ret 0)
return -1;
@@ -1120,7 +1122,7 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int
*lastCpu, long *vm_rss,
VIR_DEBUG(Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld,
- pid, tid, usertime, systime, cpu, rss);
+ (int) pid, tid, usertime, systime, cpu, rss);
VIR_FORCE_FCLOSE(pidinfo);
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 3e1a72f..e71dc20 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -94,9 +94,10 @@ static const char *
virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNU
}
static int
-virSecurityDACSetOwnership(const char *path, int uid, int gid)
+virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
{
-VIR_INFO(Setting DAC user and group on '%s' to '%d:%d', path, uid, gid);
+VIR_INFO(Setting DAC user and group on '%s' to '%ld:%ld',
+ path, (long) uid, (long) gid);
if (chown(path, uid, gid) 0) {
struct stat sb;
@@ -111,18 +112,22 @@ virSecurityDACSetOwnership(const char *path, int uid, int
gid)
}
if (chown_errno == EOPNOTSUPP || chown_errno == EINVAL) {
-VIR_INFO(Setting user and group to '%d:%d' on '%s' not supported by
filesystem,
- uid, gid, path);
+VIR_INFO(Setting user and group to '%ld:%ld' on '%s' not
+ supported by filesystem,
+ (long) uid, (long) gid, path);
} else if (chown_errno == EPERM) {
-VIR_INFO(Setting user and group to '%d:%d' on '%s' not permitted,
- uid, gid, path);
+VIR_INFO(Setting user and group to '%ld:%ld' on '%s' not
+ permitted,
+ (long) uid, (long) gid, path);
} else if (chown_errno == EROFS) {
-VIR_INFO(Setting user and group to '%d:%d' on '%s' not possible on
readonly filesystem,
- uid, gid, path);
+VIR_INFO(Setting user and group to '%ld:%ld' on '%s' not
+ possible on readonly filesystem,
+ (long) uid, (long) gid, path);
} else {
virReportSystemError(chown_errno,
- _(unable to set user and group to
Re: [libvirt] [PATCH 1/2] build: use correct type for pid and similar types
On 03/02/2012 05:42 AM, Peter Krempa wrote:
Sorry for remembering really late to review your patch.
No problem. There's still some work to do to get things happy now that
F17 has switched cross toolchains to mingw64.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1b92aa4..2e63193 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7925,7 +7926,7 @@ virDomainDefPtr
qemuParseCommandLinePid(virCapsPtr caps,
pidfile, monConfig, monJSON)))
goto cleanup;
-if (virAsprintf(exepath, /proc/%u/exe, pid) 0) {
+if (virAsprintf(exepath, /proc/%u/exe, (int) pid) 0) {
I'd use /proc/%lld/exe and cast pid to (long long). Or at least change
%u to %d for the (int) typecast. I agree that linux does not use
long pids now, but it's nicer when the code is uniform and you used the
long long variant later on and in the 2/2 patch of this series.
I'll go with the short %d, along with a comment why it is safe.
virReportSystemError(chown_errno,
- _(unable to set user and group to
'%d:%d' on '%s'),
- uid, gid, path);
+ _(unable to set user and group to
'%ld:%ld'
+ on '%s'),
+ (long) uid, (long) gid, path);
return -1;
}
}
Again, I'd prefer long longs here to line up with other instances.
We already have a compile-time assertion that uid_t and gid_t fit in
long, and this is true for all known platforms including ming64. It is
only pid_t (and thus id_t) that is problematic. We also have other
instances of commits that just cast to long when printing uids, so if I
were to make things consistent with long long, I'd have to touch more
code. So I don't think this one is worth changing.
ACK with the mismatch of signedness of the formating string and typecast
in the first and second hunk of this reply. The other comments are just
cosmetic so I'm ok if you leave the rest as is.
Sounds like I'll just fix the first two hunks, then :)
--
Eric Blake ebl...@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/2] build: fix output of pid values
On 02/11/2012 12:55 AM, Eric Blake wrote: Nuke the last vestiges of printing pid_t values with the wrong types, at least in code compiled on mingw64. There may be other places, but for now they are only compiled on systems where the existing %d doesn't trigger gcc warnings. * src/rpc/virnetsocket.c (virNetSocketNew): Use %lld and casting, rather than assuming any particular int type for pid_t. * src/util/command.c (virCommandRunAsync, virPidWait) (virPidAbort): Likewise. (verify): Drop a now stale assertion. --- diff --git a/src/util/command.c b/src/util/command.c index a2d5f84..b752b2a 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -50,9 +49,6 @@ virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) -/* We have quite a bit of changes to make if this doesn't hold. */ -verify(sizeof(pid_t)= sizeof(int)); -2011-07-12 10:42:41 -0600 Didn't hold that long actually :) (It was added 2011-07-12 10:42:41 -0600 ) ACK, Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: prohibit cross-inclusion
On 03/02/2012 03:01 AM, Daniel P. Berrange wrote: On Thu, Mar 01, 2012 at 05:40:19PM -0700, Eric Blake wrote: Make it easier to detect invalid cross-directory includes, by adding a syntax check. The check is designed to be extensible: the default case lists only the non-driver directories, and specific directories can list a different set (for example, util/ can only use itself, network/ can only use itself, util/, or conf/). * .gnulib: Update to latest, for syntax check improvment. * cfg.mk (sc_prohibit_cross_inclusion): New check. (sc_prohibit_strncmp, sc_libvirt_unmarked_diagnostics): Simplify. --- +# Our code is divided into modular subdirectories for a reason, and +# lower-level code must not include higher-level headers. +cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.)) +cross_dirs_re=($(subst / ,/|,$(cross_dirs))) +sc_prohibit_cross_inclusion: +@for dir in $(cross_dirs); do \ + case $$dir in \ +util/) safe=util;;\ +cpu/ | locking/ | network/ | rpc/ | security/) \ + safe=($$dir|util|conf);;\ +xenapi/ | xenxs/ ) safe=($$dir|util|conf|xen);; \ +*) safe=($$dir|util|conf|cpu|network|locking|rpc|security);; \ + esac; \ + in_vc_files=^src/$$dir \ + prohibit='^# *include .$(cross_dirs_re)' \ + exclude=# *include .$$safe \ + halt='unsafe cross-directory include' \ +$(_sc_search_regexp)\ +done + # When converting an enum to a string, make sure that we track any new # elements added to the enum by using a _LAST marker. sc_require_enum_last_marker: ACK this looks good to me Thanks; pushed. Hmm, should we change things to drop the -Iutil and instead use #include util/util.h, rather than the current #include util.h, as a way to make things even more obvious which submodule various headers are coming from? But that would be a future patch, and doesn't need to hold up this one. -- Eric Blake ebl...@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] build: prohibit cross-inclusion
On Fri, Mar 02, 2012 at 06:40:57AM -0700, Eric Blake wrote: On 03/02/2012 03:01 AM, Daniel P. Berrange wrote: On Thu, Mar 01, 2012 at 05:40:19PM -0700, Eric Blake wrote: Make it easier to detect invalid cross-directory includes, by adding a syntax check. The check is designed to be extensible: the default case lists only the non-driver directories, and specific directories can list a different set (for example, util/ can only use itself, network/ can only use itself, util/, or conf/). * .gnulib: Update to latest, for syntax check improvment. * cfg.mk (sc_prohibit_cross_inclusion): New check. (sc_prohibit_strncmp, sc_libvirt_unmarked_diagnostics): Simplify. --- +# Our code is divided into modular subdirectories for a reason, and +# lower-level code must not include higher-level headers. +cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.)) +cross_dirs_re=($(subst / ,/|,$(cross_dirs))) +sc_prohibit_cross_inclusion: + @for dir in $(cross_dirs); do \ +case $$dir in \ + util/) safe=util;;\ + cpu/ | locking/ | network/ | rpc/ | security/) \ +safe=($$dir|util|conf);;\ + xenapi/ | xenxs/ ) safe=($$dir|util|conf|xen);; \ + *) safe=($$dir|util|conf|cpu|network|locking|rpc|security);; \ +esac; \ +in_vc_files=^src/$$dir \ +prohibit='^# *include .$(cross_dirs_re)' \ +exclude=# *include .$$safe \ +halt='unsafe cross-directory include' \ + $(_sc_search_regexp)\ + done + # When converting an enum to a string, make sure that we track any new # elements added to the enum by using a _LAST marker. sc_require_enum_last_marker: ACK this looks good to me Thanks; pushed. Hmm, should we change things to drop the -Iutil and instead use #include util/util.h, rather than the current #include util.h, as a way to make things even more obvious which submodule various headers are coming from? But that would be a future patch, and doesn't need to hold up this one. Yeah, I think that could be nice - we've already been doing that with some of the newer dirs like locking/ and security/. 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] build: prohibit cross-inclusion
On 03/01/2012 07:40 PM, Eric Blake wrote: Make it easier to detect invalid cross-directory includes, by adding a syntax check. The check is designed to be extensible: the default case lists only the non-driver directories, and specific directories can list a different set (for example, util/ can only use itself, network/ can only use itself, util/, or conf/). * .gnulib: Update to latest, for syntax check improvment. * cfg.mk (sc_prohibit_cross_inclusion): New check. (sc_prohibit_strncmp, sc_libvirt_unmarked_diagnostics): Simplify. --- +# Our code is divided into modular subdirectories for a reason, and +# lower-level code must not include higher-level headers. +cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.)) +cross_dirs_re=($(subst / ,/|,$(cross_dirs))) +sc_prohibit_cross_inclusion: + @for dir in $(cross_dirs); do \ + case $$dir in \ + util/) safe=util;;\ + cpu/ | locking/ | network/ | rpc/ | security/) \ + safe=($$dir|util|conf);;\ + xenapi/ | xenxs/ ) safe=($$dir|util|conf|xen);; \ + *) safe=($$dir|util|conf|cpu|network|locking|rpc|security);; \ + esac; \ + in_vc_files=^src/$$dir \ + prohibit='^# *include .$(cross_dirs_re)' \ + exclude=# *include .$$safe \ + halt='unsafe cross-directory include' \ Should this maybe say prohibited instead of un-safe? BTW, I just did a full build with the new gnulib, and tried syntax-check - it did properly catch the problem that I had removed in the patch that started this discussion. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] build: fix output of pid values
On 03/02/2012 06:00 AM, Peter Krempa wrote: On 02/11/2012 12:55 AM, Eric Blake wrote: Nuke the last vestiges of printing pid_t values with the wrong types, at least in code compiled on mingw64. There may be other places, but for now they are only compiled on systems where the existing %d doesn't trigger gcc warnings. * src/rpc/virnetsocket.c (virNetSocketNew): Use %lld and casting, rather than assuming any particular int type for pid_t. * src/util/command.c (virCommandRunAsync, virPidWait) (virPidAbort): Likewise. (verify): Drop a now stale assertion. --- diff --git a/src/util/command.c b/src/util/command.c index a2d5f84..b752b2a 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -50,9 +49,6 @@ virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) -/* We have quite a bit of changes to make if this doesn't hold. */ -verify(sizeof(pid_t)= sizeof(int)); -2011-07-12 10:42:41 -0600 Didn't hold that long actually :) (It was added 2011-07-12 10:42:41 -0600 ) ACK, Thanks; series pushed with 1/2 tweaked as mentioned. -- Eric Blake ebl...@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] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On Thu, 2012-03-01 at 13:02 -0500, Laine Stump wrote: In the case of hostdev though, there is not necessarily any netdev driver at all in the host (and thus no linkdev to attach a macvtap to), certainly not after it's attached to the guest - control of the PCI device is given over to the guest. So is the problem here that 802.1QbX stuff can only work if there's an associated macvtap device? Although it might be possible to temporarily create a macvtap device and attach it to the PCI device's netdev driver prior to passing it through, that would only work if a netdev driver was bound to the PCI device (which isn't always the case, especially for SRIOV VFs), yet that netdev driver would then immediately need to be unbound prior to assigning the device to the guest, and most likely that would kill the macvtap device; even if the setup done using that macvtap device wasn't undone in the process, would it be possible to undo it later when the guest terminates (or the device is detached from the guest)? I wondered how the complete XML fragment for Qbh would look like and came up with the following: interface type='hostdev' source dev='eth0' mode='private'/ mac address='52:54:00:8b:c9:51' virtualport type='802.1Qbh' parameters profileid='finance'/ /virtualport /interface Can someone confirm? For Qbg, we would need then something like this: interface type='hostdev' source dev='eth0' mode='vepa'/ mac address='52:54:00:8b:c9:51' virtualport type=802.1Qbg parameters managerid=11 typeid=1193047 typeidversion=2 instanceid=09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f vlanid=2/ /virtualport /interface to be of any use. Note the additional vlanid attribute. The semantic would be that the host establishes a Qbg association for (managerid, typeid, typeidversion, instanceid, vlanid) and that the VM would need to add the correct VLAN tag in order to be able to communicate. Does that make sense? Best regards, Gerhard Stenzel, Hybrid Technologies, LTC --- IBM Deutschland Research Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] daemon: Remove deprecated HAL from init script dependencies
The init script for the daemon requests to start HAL although it has been deprecated long time ago. This patch removes the dependency. --- daemon/libvirtd.init.in |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.init.in b/daemon/libvirtd.init.in index 3c49b1f..f66ddad 100644 --- a/daemon/libvirtd.init.in +++ b/daemon/libvirtd.init.in @@ -8,7 +8,6 @@ # Required-Start: $network messagebus # Should-Start: $named # Should-Start: xend -# Should-Start: hal # Should-Start: avahi-daemon # Required-Stop: $network messagebus # Should-Stop: $named -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] daemon: Remove deprecated HAL from init script dependencies
On 03/02/2012 08:33 AM, Peter Krempa wrote: The init script for the daemon requests to start HAL although it has been deprecated long time ago. This patch removes the dependency. --- daemon/libvirtd.init.in |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.init.in b/daemon/libvirtd.init.in index 3c49b1f..f66ddad 100644 --- a/daemon/libvirtd.init.in +++ b/daemon/libvirtd.init.in @@ -8,7 +8,6 @@ # Required-Start: $network messagebus # Should-Start: $named # Should-Start: xend -# Should-Start: hal Should we make this a configure decision, as in: configure.ac: if test $with_hal = yes; then AC_SUBST([INIT_HAL], [Should-Start: hal]) else AC_SUBST([INIT_HAL], []) fi libvirtd.init.in: # @INIT_HAL@ Then again, in typing it out, I think that's overkill. So ACK to your patch, as-is. -- Eric Blake ebl...@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] daemon: Remove deprecated HAL from init script dependencies
On 03/02/2012 04:38 PM, Eric Blake wrote: On 03/02/2012 08:33 AM, Peter Krempa wrote: The init script for the daemon requests to start HAL although it has been deprecated long time ago. This patch removes the dependency. --- daemon/libvirtd.init.in |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.init.in b/daemon/libvirtd.init.in index 3c49b1f..f66ddad 100644 --- a/daemon/libvirtd.init.in +++ b/daemon/libvirtd.init.in @@ -8,7 +8,6 @@ # Required-Start: $network messagebus # Should-Start: $named # Should-Start: xend -# Should-Start: hal Should we make this a configure decision, as in: I was considering this option, but I don't think that anybody will ever try to use HAL with recent libvirt again. configure.ac: if test $with_hal = yes; then AC_SUBST([INIT_HAL], [Should-Start: hal]) else AC_SUBST([INIT_HAL], []) fi libvirtd.init.in: # @INIT_HAL@ Then again, in typing it out, I think that's overkill. So ACK to your patch, as-is. Thanks; pushed. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 03/02/2012 09:12 AM, Gerhard Stenzel wrote: On Thu, 2012-03-01 at 13:02 -0500, Laine Stump wrote: In the case of hostdev though, there is not necessarily any netdev driver at all in the host (and thus no linkdev to attach a macvtap to), certainly not after it's attached to the guest - control of the PCI device is given over to the guest. So is the problem here that 802.1QbX stuff can only work if there's an associated macvtap device? Although it might be possible to temporarily create a macvtap device and attach it to the PCI device's netdev driver prior to passing it through, that would only work if a netdev driver was bound to the PCI device (which isn't always the case, especially for SRIOV VFs), yet that netdev driver would then immediately need to be unbound prior to assigning the device to the guest, and most likely that would kill the macvtap device; even if the setup done using that macvtap device wasn't undone in the process, would it be possible to undo it later when the guest terminates (or the device is detached from the guest)? I wondered how the complete XML fragment for Qbh would look like and came up with the following: interface type='hostdev' source dev='eth0' mode='private'/ 1) Currently it requires a PCI address (although I plan to add the ability to accept a netdev name and automatically convert it to a PCI address): source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source 2) I guess I have been misunderestimating the level of symbiosis between macvtap and 802.1QbX. I had thought that the private vs. vepa thing was related to whether or not macvtap could (or couldn't) share the physical device and (when sharing was allowed) whether or not it allowed multiple macvtap devices connected to the same physical to see traffic from each other. This assumption led me to believe that in the case of a PCI passthrough device, where there is obviously no sharing (or macvtap device), these different modes were irrelevant, and all that was needed was the information in virtualport. What I *think* I'm understanding from this discussion is that 1) in order for a virtual port association to happen, a macvtap interface must exist, and the association is done wrt that macvtap device *not* the physical device, or even the VF, and 2) knowing the information in virtualport (along with knowing that the physical device is not being shared) is not enough information to properly perform an associate operation. Is this correct? If that's the case, then there are some basic assumptions made here that are incorrect, and we will need to either change the lower level code to somehow accomplish a port associate without a macvtap interface, or we will need to pull some kind of trickery, possibly adding a macvtap interface to the PF to be used as a proxy to do the ASSOCIATE for the VF (will that even work? In particular, will it work if multiple VFs need to operate in one of the exclusive modes where no sharing of physical device is permitted?) Or maybe I'm still not understanding... mac address='52:54:00:8b:c9:51' virtualport type='802.1Qbh' parameters profileid='finance'/ /virtualport /interface Can someone confirm? For Qbg, we would need then something like this: interface type='hostdev' source dev='eth0' mode='vepa'/ mac address='52:54:00:8b:c9:51' virtualport type=802.1Qbg parameters managerid=11 typeid=1193047 typeidversion=2 instanceid=09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f vlanid=2/ /virtualport /interface to be of any use. Again, my knowledge is insufficient to understand - why was a vlanid not necessary before when we were dealing with a hostside macvtap tied to a guest-side emulated netdev, and why is it necessary now that we want to just passthrough the PCI device to the guest? Note the additional vlanid attribute. The semantic would be that the host establishes a Qbg association for (managerid, typeid, typeidversion, instanceid, vlanid) and that the VM would need to add the correct VLAN tag in order to be able to communicate. So adding the VLAN tag has in the past been done by the macvtap interface? Where did it learn the vlanid from? Definitely if the packets need to leave the host with a VLAN tag, in PCI passthrough mode that will need to be done by the guest OS, since the host will be unable to get its hands on the packets. Once that's the case, does it maybe make more sense to just leave *everything* up to the guest OS - do a PCI passthrough of the device (maybe setting the MAC address) and let the guest do the port associate etc. too? (Another way of saying this - at this point, shouldn't we just admit that transparent hostside support of VEPA (or any other protocol that requires data packets to be modified) using PCI passthrough by definition is not possible, and therefore isn't supported?) Or am I just misunderstanding again? Does that make sense?
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 03/02/2012 10:52 AM, Laine Stump wrote: On 03/02/2012 09:12 AM, Gerhard Stenzel wrote: On Thu, 2012-03-01 at 13:02 -0500, Laine Stump wrote: In the case of hostdev though, there is not necessarily any netdev driver at all in the host (and thus no linkdev to attach a macvtap to), certainly not after it's attached to the guest - control of the PCI device is given over to the guest. So is the problem here that 802.1QbX stuff can only work if there's an associated macvtap device? Although it might be possible to temporarily create a macvtap device and attach it to the PCI device's netdev driver prior to passing it through, that would only work if a netdev driver was bound to the PCI device (which isn't always the case, especially for SRIOV VFs), yet that netdev driver would then immediately need to be unbound prior to assigning the device to the guest, and most likely that would kill the macvtap device; even if the setup done using that macvtap device wasn't undone in the process, would it be possible to undo it later when the guest terminates (or the device is detached from the guest)? I wondered how the complete XML fragment for Qbh would look like and came up with the following: interface type='hostdev' source dev='eth0' mode='private'/ 1) Currently it requires a PCI address (although I plan to add the ability to accept a netdev name and automatically convert it to a PCI address): source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source 2) I guess I have been misunderestimating the level of symbiosis between macvtap and 802.1QbX. I had thought that the private vs. vepa thing was related to whether or not macvtap could (or couldn't) share the physical device and (when sharing was allowed) whether or not it allowed multiple macvtap devices connected to the same physical to see traffic from each other. This assumption led me to believe that in the case of a PCI passthrough device, where there is obviously no sharing (or macvtap device), these different modes were irrelevant, and all that was needed was the information invirtualport. What I *think* I'm understanding from this discussion is that 1) in order for a virtual port association to happen, a macvtap interface must exist, and the association is done wrt that macvtap device *not* the physical device, or even the VF, and 2) knowing the information in Well, another aspect of 802.1Qbg versus Qbh is that 802.1Qbg needs an external daemon, lldpad, to setup the switch. 802.1Qbh presumably does 'all it needs' by talking to the firmware on the ethernet card... The protocol between libvirt and lldpad currently requires the transfer of an interface. So this protocol would have to be extended to 'somehow' support a raw PIC bus/device/function. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Attach vm-uuid to Open vSwitch interfaces.
On Mar 1, 2012, at 11:51 PM, Ansis Atteka wrote:
This patch will allow OpenFlow controllers to identify which interface
belongs to a particular VM by using the Domain UUID.
ovs-vsctl get Interface vnet0 external_ids
{attached-mac=52:54:00:8C:55:2C,
iface-id=83ce45d6-3639-096e-ab3c-21f66a05f7fa, iface-status=active,
vm-uuid=142a90a7-0acc-ab92-511c-586f12da8851}
This patch looks good to me Ansis, and it will be handy to have the VM UUID
available in the OVS DB for connected ports. Nice work!
Thanks,
Kyle
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On Fri, 2012-03-02 at 10:52 -0500, Laine Stump wrote: Again, my knowledge is insufficient to understand - why was a vlanid not necessary before when we were dealing with a hostside macvtap tied to a guest-side emulated netdev, and why is it necessary now that we want to just passthrough the PCI device to the guest? Note the additional vlanid attribute. The semantic would be that the host establishes a Qbg association for (managerid, typeid, typeidversion, instanceid, vlanid) and that the VM would need to add the correct VLAN tag in order to be able to communicate. So adding the VLAN tag has in the past been done by the macvtap interface? Where did it learn the vlanid from? (Many questions for which I will need some time ..) Let me answer the simple ones first: If you look here http://libvirt.org/formatdomain.html: devices interface type='direct'/ ... interface type='direct' source dev='eth0.2' mode='vepa'/ virtualport type=802.1Qbg parameters managerid=11 typeid=1193047 typeidversion=2 instanceid=09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f/ /virtualport /interface /devices ... In this example, the macvtap interface will be created on top of the VLAN interface 2 on top of eth0. The Qbg switch needs this information: (managerid, typeid, typeidversion, instanceid, vlanid) macvtap/VEPA does not need the the VLAN to work, but Qbg does. So for PCI passthrough, if the host does the association, it has to know which VLANID to associate, but the guest has to add the VLAN tags. Definitely if the packets need to leave the host with a VLAN tag, in PCI passthrough mode that will need to be done by the guest OS, since the host will be unable to get its hands on the packets. Once that's the case, does it maybe make more sense to just leave *everything* up to the guest OS - do a PCI passthrough of the device (maybe setting the MAC address) and let the guest do the port associate etc. too? (Another way of saying this - at this point, shouldn't we just admit that transparent hostside support of VEPA (or any other protocol that requires data packets to be modified) using PCI passthrough by definition is not possible, and therefore isn't supported?) Letting the guest do the association is an option, which should work already (even if noone probably tested it yet), but the question is really how much control should the host have vs the guest. There are definitely scenarios thinkable where the host should do the association. Best regards, Gerhard Stenzel, Hybrid Technologies, LTC --- IBM Deutschland Research Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 03/02/2012 11:37 AM, Gerhard Stenzel wrote: Letting the guest do the association is an option, which should work already (even if noone probably tested it yet), but the question is really how much control should the host have vs the guest. There are definitely scenarios thinkable where the host should do the association. I think an applicable scenario is where the infrastructure provider doesn't really know the guest user and needs to have 'mandatory access control' over the configuration of the infrastructure and yet wants to use the pass-through mode for better throughput. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Support mac and port profile for interface type='hostdev'
On 03/01/2012 04:02 AM, Roopa Prabhu wrote: This patch series is based on laines patches to support interface type='hostdev'. https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html It support to set mac and port profile on an interface of type hostdev. * If virtualport is specified, the existing virtual port functions are called to set mac, vlan and port profile. I'm unable to test that part, as I don't have any 802.1QbX capable switches (and it sounds like the design is problematic anyway.) * If virtualport is not specified and device is a sriov virtual function, - mac is set using IFLA_VF_MAC Success!! I tried this for VFs that have a netdev driver attached, and VFs that don't, and it behaved properly in both cases - when the guest is started, the MAC address is set properly for the guest to use, and when the guest is stopped, the MAC address of that VF is restored to its original value (implying that your code to save the old MAC address works properly). * If virtualport is not specified and device is a non-sriov virtual function, - mac is set using existing SIOCGIFHWADDR (This requires that the netdev be present on the host before starting the VM) This one has a problem, at least with my non-sriov hardware (which happens to be the onboard NetXtreme device of a Thinkstation, using the tg3 driver) it appears the MAC address gets reset to its original setting at some point after libvirt changes it. To help understand what happens - assume the device's original MAC address is o:o:o:o:o:o, and my xml looks like this: interface type='hostdev' managed='yes' mac address='n:n:n:n:n:n'/ ... /interface When the guest boots up, ifconfig shows there is an interface with mac address o:o:o:o:o:o. Additionally, if I manually change the mac address to p:p:p:p:p:p on the host before starting the guest, when the guest boots, ifconfig shows the mac address as... o:o:o:o:o:o. So, whether or not libvirt is successfully setting the mac address, it's getting reset (probably by the card's firmware). So perhaps this is another case of wanting to do something that just isn't possible, and the way out is to simply generate an error on domain startup if the netdev being passed through isn't a VF? This series implements the below : 01/4 pci: Add two new pci util pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile 02/4 virtnetdev: Add support functions for mac and portprofile associations on a hostdev 03/4 virnetdevvportprofile: Changes to support portprofiles for hostdevs 04/4 qemu_hostdev: Add support to install port profile and mac address on hostdev Stefan Berger is CC'ed for 802.1Qbg changes in patch 03/4. Current code for 802.1Qbg uses macvtap ifname. And for network interfaces with type=hostdev a macvtap ifname does not exist. This patch just adds a null check for ifname in 802.1Qbg port profile handling code. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Rebélate by self-management, first project of free software by which we bet all / Rebélate por la autogestión, primer proyecto de software libre por el que apostamos todas
Inglés : Many already we have contributed to the first project of free software dedicated to self-management in this campaign of collective financing, it collaborates and it spreads!/ Beginning campaign collective financing http://www.goteo.org/project/rebelaos-publicacion-por-la-autogestion?lang=en Login to enter with user of social networks and for would register in Goteo : http://www.goteo.org/user/login?lang=en Rebelaos! Publication by self-management A massive publication that floods the public transport, the work centers, the parks, the consumption centers, by means of distribution of 500,000 gratuitous units, acting simultaneously in all sides and nowhere. We announce the main tool of a vestibule Web for the management of self-sustaining resources by means of Drupal, in addition in the publication there will be an article dedicated to free software, hardware, It is being prepared in inglès, the machinery You can see more details in the index of the publication https://n-1.cc/pg/file/read/1151902/indexresumen-de-los-contenidos-pdf . A computer system that allows us to share resources in all the scopes of our life so that we do not have to generate means different for each subject nor for each territory. A point of contact digitalis to generate projects of life outside Capitalism and to margin of the State. A tool to spread and to impel the social transformation through the resources that will set out in their contents around self-management, the autoorganización, the disobedience and the collective action. In which the capitalist system goes to the collapse, in a while immersed in a deep systemic crisis (ecological, political and economic, but mainly of values), where individual and collective of people they are being lacking of his fundamental rights, is necessary to develop a horizontal collective process where all the human beings we pruned to interact in equality of conditions and freedom. To interact means to relate to us (as much human as economically), to communicate to us, to cover our basic needs, to generate and to protect communal properties, to know and to provide collective solutions us problematic that our lives interfere. We want abrir a breach within normality in the monotonous life state-capitalist, a day anyone, that finally will not be any day. By means of this publication we try: - To drive a horizontal collective process where all and all we pruned to interact in equality of conditions and freedom. - To create communications network between the people it jeopardize with the change and arranged to act. - To find collective solutions to problematic that our lives interfere - To facilitate the access to resources that make possible self-management. - To participate in the construction of networks of mutual support, generated horizontals, asamblearias and from the base. - To publish all this information in an attractive format stops to facilitate the access to all the society. There are 15 days remaining for the upcoming March 15, the day that will come Rebelaos!, Magazine for the selfmanagement Today, we issue the cover of Rebelaos! (Castilian version) that can be displayed on the following link: https://n-1.cc/pg/file/read/1200503/portada-15-de-marzo-rebelaos The contents of the store owners to us by 15 March. Do you? Do you keep on 15 March? In addition, we have over 200 distribution nodes, distributed throughout the Spanish state. Check the map: https://afinidadrebelde.crowdmap.com/ On the other hand, the funding campaign continues to move and still have 12 days to collect the remaining 6,000 euros. We can all make a bit for all the grains of sand become a great beach on March 15. You can access the co-financing campaign: http://www.goteo.org/project/rebelaos-publicacion-por-la-autogestion Rebel Affinity group www.rebelaos.net --- Castellano: Muchos ya hemos aportado al primer proyecto de software libre dedicado a la la financiación colectiva, colabora y diffunde ! Inicio campaña financiación colectiva goteo.org www.goteo.org/project/rebelaos-publicacion-por-la-autogestion Link para registrarse en Goteo y acceder a redes sociales para colaborar en la difusín http://www.goteo.org/user/login ¡Rebelaos! Publicación por la autogestión Una publicación masiva que inunde el transporte público, los centros de trabajo, los parques, los centros de consumo, mediante la distribución de 500.000 ejemplares gratuitos, actuando simultáneamente en todos lados y en ninguna parte. Anunciamos la herramienta principal de un portal web para la gestión de recursos autogestionados mediante Drupal, además en la publicación habrá un artículo dedicado al software libre, el hardware, la maquinaria... Puedes ver más detalles en el índice de la publicación https://n-1.cc/pg/file/read/1151902/indexresumen-de-los-contenidos-pdf Un sistema infórmatico que nos permita
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 03/02/2012 11:58 AM, Stefan Berger wrote: On 03/02/2012 11:37 AM, Gerhard Stenzel wrote: Letting the guest do the association is an option, which should work already (even if noone probably tested it yet), but the question is really how much control should the host have vs the guest. There are definitely scenarios thinkable where the host should do the association. I think an applicable scenario is where the infrastructure provider doesn't really know the guest user and needs to have 'mandatory access control' over the configuration of the infrastructure and yet wants to use the pass-through mode for better throughput. And/or when the guest OS doesn't have the necessary smarts to do the association (or maybe even vlan tagging) itself. So, at the end of this, is there *any* 802.1QbX mode that can work using PCI passthrough without at least one of the following things: 1) a macvtap interface on the host. (what about my idea of attaching a macvtap interface to the PF? does that have any hint of practicality?) 2) extending the protocol for talking with lldpad to support using a raw PCI device rather than a macvtap device. 3) the guest doing vlan tagging 4) the guest doing the full 802.1QbX associate/de-associate protocol exchange itself? Nobody has said it explicitly yet (I think), but I have the impression that this problem unfortunately can't be solved by libvirt alone. If that's the case, we should state that as soon as possible so that we can table the virtualport part of these patches for the short term, and separate the mac address part to get it pushed upstream (along with the new low-level PCI utility functions), as that is very useful on its own. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On Fri, 2012-03-02 at 10:52 -0500, Laine Stump wrote: 1) Currently it requires a PCI address (although I plan to add the ability to accept a netdev name and automatically convert it to a PCI address): source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source This means the XML fragment look more like this for Qbh: interface type='hostdev' source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source mac address='52:54:00:8b:c9:51' virtualport type='802.1Qbh' parameters profileid='finance'/ /virtualport /interface and for Qbg: interface type='hostdev' source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source mac address='52:54:00:8b:c9:51' virtualport type=802.1Qbg parameters managerid=11 typeid=1193047 typeidversion=2 instanceid=09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f vlanid=2/ /virtualport /interface 2) I guess I have been misunderestimating the level of symbiosis between macvtap and 802.1QbX. I had thought that the private vs. vepa thing was related to whether or not macvtap could (or couldn't) share the physical device and (when sharing was allowed) whether or not it allowed multiple macvtap devices connected to the same physical to see traffic from each other. This assumption led me to believe that in the case of a PCI passthrough device, where there is obviously no sharing (or macvtap device), these different modes were irrelevant, and all that was needed was the information in virtualport. What I *think* I'm understanding from this discussion is that 1) in order for a virtual port association to happen, a macvtap interface must exist, and the association is done wrt that macvtap device *not* the physical device, or even the VF, and 2) knowing the information in virtualport (along with knowing that the physical device is not being shared) is not enough information to properly perform an associate operation. Is this correct? If I understand above correctly, your first assumption seems correct and my XML examples have been misleading you. If that's the case, then there are some basic assumptions made here that are incorrect, and we will need to either change the lower level code to somehow accomplish a port associate without a macvtap interface, or we will need to pull some kind of trickery, possibly adding a macvtap interface to the PF to be used as a proxy to do the ASSOCIATE for the VF (will that even work? In particular, will it work if multiple VFs need to operate in one of the exclusive modes where no sharing of physical device is permitted?) I do not know for Qbh, but for Qbg: The switch knows nothing about macvtap devices or virtual functions, what matters is the combination of (managerid, typeid, typeidversion, instanceid, vlanid) to make an association. Best regards, Gerhard Stenzel, Hybrid Technologies, LTC --- IBM Deutschland Research Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Support mac and port profile for interface type='hostdev'
On 3/2/12 11:04 AM, Laine Stump la...@laine.org wrote: On 03/01/2012 04:02 AM, Roopa Prabhu wrote: This patch series is based on laines patches to support interface type='hostdev'. https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html It support to set mac and port profile on an interface of type hostdev. * If virtualport is specified, the existing virtual port functions are called to set mac, vlan and port profile. I'm unable to test that part, as I don't have any 802.1QbX capable switches (and it sounds like the design is problematic anyway.) The design is still fine for 802.1Qbh. I have tested it. 802.1Qbh does not need a macvtap device. For 802.1Qbg in v2 (was planning on pushing it the next hr), I have put a check for 802.1Qbg and hostdevs and fail as suggested by stefan. * If virtualport is not specified and device is a sriov virtual function, - mac is set using IFLA_VF_MAC Success!! I tried this for VFs that have a netdev driver attached, and VFs that don't, and it behaved properly in both cases - when the guest is started, the MAC address is set properly for the guest to use, and when the guest is stopped, the MAC address of that VF is restored to its original value (implying that your code to save the old MAC address works properly). Nice. Thanks for trying it out. * If virtualport is not specified and device is a non-sriov virtual function, - mac is set using existing SIOCGIFHWADDR (This requires that the netdev be present on the host before starting the VM) This one has a problem, at least with my non-sriov hardware (which happens to be the onboard NetXtreme device of a Thinkstation, using the tg3 driver) it appears the MAC address gets reset to its original setting at some point after libvirt changes it. To help understand what happens - assume the device's original MAC address is o:o:o:o:o:o, and my xml looks like this: interface type='hostdev' managed='yes' mac address='n:n:n:n:n:n'/ ... /interface When the guest boots up, ifconfig shows there is an interface with mac address o:o:o:o:o:o. Additionally, if I manually change the mac address to p:p:p:p:p:p on the host before starting the guest, when the guest boots, ifconfig shows the mac address as... o:o:o:o:o:o. So, whether or not libvirt is successfully setting the mac address, it's getting reset (probably by the card's firmware). Yes this I kind of expected. It depends on the driver. I thought there are some drivers that remember the mac address set by SIOCGIFHWADDR. But my assumption was totally based on the fact that we wanted to add support for all devices. Having the code there just means we try to set the device mac. It takes effect only if the hw supports it. So perhaps this is another case of wanting to do something that just isn't possible, and the way out is to simply generate an error on domain startup if the netdev being passed through isn't a VF? We can do this too. Only support it for sriov vf's. Thanks, Roopa This series implements the below : 01/4 pci: Add two new pci util pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile 02/4 virtnetdev: Add support functions for mac and portprofile associations on a hostdev 03/4 virnetdevvportprofile: Changes to support portprofiles for hostdevs 04/4 qemu_hostdev: Add support to install port profile and mac address on hostdev Stefan Berger is CC'ed for 802.1Qbg changes in patch 03/4. Current code for 802.1Qbg uses macvtap ifname. And for network interfaces with type=hostdev a macvtap ifname does not exist. This patch just adds a null check for ifname in 802.1Qbg port profile handling code. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/2/12 11:27 AM, Laine Stump la...@laine.org wrote: On 03/02/2012 11:58 AM, Stefan Berger wrote: On 03/02/2012 11:37 AM, Gerhard Stenzel wrote: Letting the guest do the association is an option, which should work already (even if noone probably tested it yet), but the question is really how much control should the host have vs the guest. There are definitely scenarios thinkable where the host should do the association. I think an applicable scenario is where the infrastructure provider doesn't really know the guest user and needs to have 'mandatory access control' over the configuration of the infrastructure and yet wants to use the pass-through mode for better throughput. And/or when the guest OS doesn't have the necessary smarts to do the association (or maybe even vlan tagging) itself. So, at the end of this, is there *any* 802.1QbX mode that can work using PCI passthrough without at least one of the following things: 1) a macvtap interface on the host. (what about my idea of attaching a macvtap interface to the PF? does that have any hint of practicality?) 2) extending the protocol for talking with lldpad to support using a raw PCI device rather than a macvtap device. 3) the guest doing vlan tagging 4) the guest doing the full 802.1QbX associate/de-associate protocol exchange itself? Nobody has said it explicitly yet (I think), but I have the impression that this problem unfortunately can't be solved by libvirt alone. If that's the case, we should state that as soon as possible so that we can table the virtualport part of these patches for the short term, and separate the mac address part to get it pushed upstream (along with the new low-level PCI utility functions), as that is very useful on its own. Laine, The patches I submitted for virtualport 802.1Qbh work just fine. I submitted these patches mainly to get the virtualport working for 802.1Qbh. Because we pass the port profile via the PF to fw and then to the switch. The driver in the guest just comes up on the VF and uses the already associated VF. We do the port profile association from the host because of security reasons. Instead of giving control to the guest OS. And as you can see in the patches, for 802.1Qbh port profile association on the hostdev is not much different from port profile association in the macvtap case. Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On Fri, 2012-03-02 at 14:27 -0500, Laine Stump wrote: So, at the end of this, is there *any* 802.1QbX mode that can work using PCI passthrough without at least one of the following things: 1) a macvtap interface on the host. (what about my idea of attaching a macvtap interface to the PF? does that have any hint of practicality?) 2) extending the protocol for talking with lldpad to support using a raw PCI device rather than a macvtap device. 3) the guest doing vlan tagging 4) the guest doing the full 802.1QbX associate/de-associate protocol exchange itself? Nobody has said it explicitly yet (I think), but I have the impression that this problem unfortunately can't be solved by libvirt alone. If that's the case, we should state that as soon as possible so that we can table the virtualport part of these patches for the short term, and separate the mac address part to get it pushed upstream (along with the new low-level PCI utility functions), as that is very useful on its own. I am not sure I can follow the conclusion that this can not be solved in libvirt alone. Qbg: For the macvtap case, the macvtap device is attached to the underlying physical interface and this is where the association request is sent to, via lldpad. For the PCI passthrough case, the same must be possible, assuming the physical interface can be concluded from the PCI device and the VLAN information is provided. Or do I miss something? Best regards, Gerhard Stenzel, Hybrid Technologies, LTC --- IBM Deutschland Research Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/2/12 9:21 AM, Gerhard Stenzel gsten...@linux.vnet.ibm.com wrote: On Fri, 2012-03-02 at 14:27 -0500, Laine Stump wrote: So, at the end of this, is there *any* 802.1QbX mode that can work using PCI passthrough without at least one of the following things: 1) a macvtap interface on the host. (what about my idea of attaching a macvtap interface to the PF? does that have any hint of practicality?) 2) extending the protocol for talking with lldpad to support using a raw PCI device rather than a macvtap device. 3) the guest doing vlan tagging 4) the guest doing the full 802.1QbX associate/de-associate protocol exchange itself? Nobody has said it explicitly yet (I think), but I have the impression that this problem unfortunately can't be solved by libvirt alone. If that's the case, we should state that as soon as possible so that we can table the virtualport part of these patches for the short term, and separate the mac address part to get it pushed upstream (along with the new low-level PCI utility functions), as that is very useful on its own. I am not sure I can follow the conclusion that this can not be solved in libvirt alone. Qbg: For the macvtap case, the macvtap device is attached to the underlying physical interface and this is where the association request is sent to, via lldpad. For the PCI passthrough case, the same must be possible, assuming the physical interface can be concluded from the PCI device and the VLAN information is provided. Or do I miss something? Wondering if we simplify this to only support sriov devices and for 802.1Qbg all config can be done via the PF netdevice. Am assuming the vlan info has to only reach lldpad daemon and you don't really need a vlan device on host. In which case, your new xml syntax with vlanid should work I think. - we send the mac, vlan and port profile info to lldpad via the PF with vf index using the current IFLA_VF_MAC and IFLA_VF_PORT. No ? Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 03/02/2012 12:05 PM, Gerhard Stenzel wrote: On Fri, 2012-03-02 at 10:52 -0500, Laine Stump wrote: 1) Currently it requires a PCI address (although I plan to add the ability to accept a netdev name and automatically convert it to a PCI address): source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source This means the XML fragment look more like this for Qbh: interface type='hostdev' source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source mac address='52:54:00:8b:c9:51' virtualport type='802.1Qbh' parameters profileid='finance'/ /virtualport /interface and for Qbg: interface type='hostdev' source address type='pci' domain='0' bus='0' slot='6' function='0'/ /source mac address='52:54:00:8b:c9:51' virtualport type=802.1Qbg parameters managerid=11 typeid=1193047 typeidversion=2 instanceid=09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f vlanid=2/ /virtualport /interface So vlanid is the new part? In the case we have no macvtap device we would have to find the vlanid in the XML and could convey that to lldpad directly (rather than determining it by walking the stack of interfaces as we do now) along with an ifindex of '0' or '-1' (?). If lldpad never really looked at the ifindex or ifname we sent it via the netlink message then this shouldn't be a problem to support, apart from having to configure the guest to create a vlan interface of course. So I guess the other set of parameters were not applied to the VM's traffic to allow VM A using vlanid 2 to be distinguishable from VM B using vlanid 2 as well on the same host? If this is all true, then at least the code path creating the association should be easy to adapt... Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 03/02/2012 03:16 PM, Roopa Prabhu wrote: On 3/2/12 11:27 AM, Laine Stump la...@laine.org wrote: On 03/02/2012 11:58 AM, Stefan Berger wrote: On 03/02/2012 11:37 AM, Gerhard Stenzel wrote: Letting the guest do the association is an option, which should work already (even if noone probably tested it yet), but the question is really how much control should the host have vs the guest. There are definitely scenarios thinkable where the host should do the association. I think an applicable scenario is where the infrastructure provider doesn't really know the guest user and needs to have 'mandatory access control' over the configuration of the infrastructure and yet wants to use the pass-through mode for better throughput. And/or when the guest OS doesn't have the necessary smarts to do the association (or maybe even vlan tagging) itself. So, at the end of this, is there *any* 802.1QbX mode that can work using PCI passthrough without at least one of the following things: 1) a macvtap interface on the host. (what about my idea of attaching a macvtap interface to the PF? does that have any hint of practicality?) 2) extending the protocol for talking with lldpad to support using a raw PCI device rather than a macvtap device. 3) the guest doing vlan tagging 4) the guest doing the full 802.1QbX associate/de-associate protocol exchange itself? Nobody has said it explicitly yet (I think), but I have the impression that this problem unfortunately can't be solved by libvirt alone. If that's the case, we should state that as soon as possible so that we can table the virtualport part of these patches for the short term, and separate the mac address part to get it pushed upstream (along with the new low-level PCI utility functions), as that is very useful on its own. Laine, The patches I submitted for virtualport 802.1Qbh work just fine. Yeah, I'm not sure how I lost sight of the fact that you said your testing had gone okay - I guess too little sleep. So I read too much into the discussion, and it's just Qbg that has these restrictions. Good! Pay no attention to my alarmism :-) I submitted these patches mainly to get the virtualport working for 802.1Qbh. Because we pass the port profile via the PF to fw and then to the switch. The driver in the guest just comes up on the VF and uses the already associated VF. Right. That's pretty much how I've always been assuming it worked for all of these modes. I guess I should spend more time at lower levels. We do the port profile association from the host because of security reasons. Instead of giving control to the guest OS. And as you can see in the patches, for 802.1Qbh port profile association on the hostdev is not much different from port profile association in the macvtap case. Okay, then in the end these patches will support 802.1Qbh virtualport setting, as well as setting the MAC address (but only for SRIOV-capable devices). And any future support for 802.1Qbg would require both some extra support outside libvirt, as well as specifying the vlanid in the config, and requiring the guest to setup VLAN tagging. Did I get it right now? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rpc: Fix client crash on connection close
A multi-threaded client with event loop may crash if one of its threads
closes a connection while event loop is in the middle of sending
keep-alive message (either request or response). The right place for it
is inside virNetClientIOEventLoop() between poll() and
virNetClientLock(). We should only close a connection directly if no-one
is using it and defer the closing to the last user otherwise. So far we
only did so if the close was initiated by keep-alive timeout.
---
src/rpc/virnetclient.c | 18 --
1 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 167fbf6..c2b901d 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -108,8 +108,6 @@ struct _virNetClient {
};
-static void virNetClientRequestClose(virNetClientPtr client);
-
static void virNetClientLock(virNetClientPtr client)
{
virMutexLock(client-lock);
@@ -253,7 +251,7 @@ virNetClientKeepAliveStart(virNetClientPtr client,
static void
virNetClientKeepAliveDeadCB(void *opaque)
{
-virNetClientRequestClose(opaque);
+virNetClientClose(opaque);
}
static int
@@ -512,19 +510,11 @@ virNetClientCloseLocked(virNetClientPtr client)
void virNetClientClose(virNetClientPtr client)
{
-if (!client)
-return;
-
-virNetClientLock(client);
-virNetClientCloseLocked(client);
-virNetClientUnlock(client);
-}
-
-static void
-virNetClientRequestClose(virNetClientPtr client)
-{
VIR_DEBUG(client=%p, client);
+if (!client)
+return;
+
virNetClientLock(client);
/* If there is a thread polling for data on the socket, set wantClose flag
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Support mac and port profile for interface type='hostdev'
On 03/02/2012 03:03 PM, Roopa Prabhu wrote: On 3/2/12 11:04 AM, Laine Stump la...@laine.org wrote: On 03/01/2012 04:02 AM, Roopa Prabhu wrote: This patch series is based on laines patches to support interface type='hostdev'. https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html It support to set mac and port profile on an interface of type hostdev. * If virtualport is specified, the existing virtual port functions are called to set mac, vlan and port profile. I'm unable to test that part, as I don't have any 802.1QbX capable switches (and it sounds like the design is problematic anyway.) The design is still fine for 802.1Qbh. Yes, my apologies for misinterpreting all the exchanges. I have tested it. 802.1Qbh does not need a macvtap device. For 802.1Qbg in v2 (was planning on pushing it the next hr), I'll try to review it as quickly as possible. I have put a check for 802.1Qbg and hostdevs and fail as suggested by stefan. I'm quickly learning that I understood much less about 802.1QbX (and in particular, how it's been implemented) than I thought! (Fortunately, as a result of all this, I think I now understand it a bit better). * If virtualport is not specified and device is a sriov virtual function, - mac is set using IFLA_VF_MAC Success!! I tried this for VFs that have a netdev driver attached, and VFs that don't, and it behaved properly in both cases - when the guest is started, the MAC address is set properly for the guest to use, and when the guest is stopped, the MAC address of that VF is restored to its original value (implying that your code to save the old MAC address works properly). Nice. Thanks for trying it out. * If virtualport is not specified and device is a non-sriov virtual function, - mac is set using existing SIOCGIFHWADDR (This requires that the netdev be present on the host before starting the VM) This one has a problem, at least with my non-sriov hardware (which happens to be the onboard NetXtreme device of a Thinkstation, using the tg3 driver) it appears the MAC address gets reset to its original setting at some point after libvirt changes it. To help understand what happens - assume the device's original MAC address is o:o:o:o:o:o, and my xml looks like this: interface type='hostdev' managed='yes' mac address='n:n:n:n:n:n'/ ... /interface When the guest boots up, ifconfig shows there is an interface with mac address o:o:o:o:o:o. Additionally, if I manually change the mac address to p:p:p:p:p:p on the host before starting the guest, when the guest boots, ifconfig shows the mac address as... o:o:o:o:o:o. So, whether or not libvirt is successfully setting the mac address, it's getting reset (probably by the card's firmware). Yes this I kind of expected. It depends on the driver. I thought there are some drivers that remember the mac address set by SIOCGIFHWADDR. But my assumption was totally based on the fact that we wanted to add support for all devices. Having the code there just means we try to set the device mac. It takes effect only if the hw supports it. If there was just a way to determine that at runtime, but it seems that by the time the MAC address has been reset, we are no longer able to call the ioctl to check the address :-( So perhaps this is another case of wanting to do something that just isn't possible, and the way out is to simply generate an error on domain startup if the netdev being passed through isn't a VF? We can do this too. Only support it for sriov vf's. Yes, if it's going to silently do the wrong thing, maybe we should leave the SIOCGIFHWADDR code in there for reference, but also add a failure condition if the card isn't SRIOV. (I guess this means my effort to make sure USB ethernets were also supported was kind of pointless, since they're sure to have the same problems :-P Mostly I only included that support to promote code sharing and consistency, though, so I'm not really disappointed.) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/2/12 12:45 PM, Laine Stump la...@laine.org wrote: On 03/02/2012 03:16 PM, Roopa Prabhu wrote: On 3/2/12 11:27 AM, Laine Stump la...@laine.org wrote: On 03/02/2012 11:58 AM, Stefan Berger wrote: On 03/02/2012 11:37 AM, Gerhard Stenzel wrote: Letting the guest do the association is an option, which should work already (even if noone probably tested it yet), but the question is really how much control should the host have vs the guest. There are definitely scenarios thinkable where the host should do the association. I think an applicable scenario is where the infrastructure provider doesn't really know the guest user and needs to have 'mandatory access control' over the configuration of the infrastructure and yet wants to use the pass-through mode for better throughput. And/or when the guest OS doesn't have the necessary smarts to do the association (or maybe even vlan tagging) itself. So, at the end of this, is there *any* 802.1QbX mode that can work using PCI passthrough without at least one of the following things: 1) a macvtap interface on the host. (what about my idea of attaching a macvtap interface to the PF? does that have any hint of practicality?) 2) extending the protocol for talking with lldpad to support using a raw PCI device rather than a macvtap device. 3) the guest doing vlan tagging 4) the guest doing the full 802.1QbX associate/de-associate protocol exchange itself? Nobody has said it explicitly yet (I think), but I have the impression that this problem unfortunately can't be solved by libvirt alone. If that's the case, we should state that as soon as possible so that we can table the virtualport part of these patches for the short term, and separate the mac address part to get it pushed upstream (along with the new low-level PCI utility functions), as that is very useful on its own. Laine, The patches I submitted for virtualport 802.1Qbh work just fine. Yeah, I'm not sure how I lost sight of the fact that you said your testing had gone okay - I guess too little sleep. So I read too much into the discussion, and it's just Qbg that has these restrictions. Good! Pay no attention to my alarmism :-) No problem :) I submitted these patches mainly to get the virtualport working for 802.1Qbh. Because we pass the port profile via the PF to fw and then to the switch. The driver in the guest just comes up on the VF and uses the already associated VF. Right. That's pretty much how I've always been assuming it worked for all of these modes. I guess I should spend more time at lower levels. We do the port profile association from the host because of security reasons. Instead of giving control to the guest OS. And as you can see in the patches, for 802.1Qbh port profile association on the hostdev is not much different from port profile association in the macvtap case. Okay, then in the end these patches will support 802.1Qbh virtualport setting, as well as setting the MAC address (but only for SRIOV-capable devices). And any future support for 802.1Qbg would require both some extra support outside libvirt, as well as specifying the vlanid in the config, and requiring the guest to setup VLAN tagging. Did I get it right now? Yes that seems right. I think we don't need to call out the setup in the guest. Its common for all modes. Host provisions the vlans (ie configures the switch port with that vlan etc). This is the step that libvirt does. And guest does his own setup for vlan devices if needed. Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On Fri, 2012-03-02 at 15:45 -0500, Laine Stump wrote: Okay, then in the end these patches will support 802.1Qbh virtualport setting, as well as setting the MAC address (but only for SRIOV-capable devices). And any future support for 802.1Qbg would require both some extra support outside libvirt, as well as specifying the vlanid in the config, and requiring the guest to setup VLAN tagging. Did I get it right now? Not sure, we need anything else for Qbg in addition to some changes in libvirt and vlan tagging in the guest. But, I think we are converging that the Qbh part looks okay and the Qbg part can be added later, if necessary. Best regards, Gerhard Stenzel, Hybrid Technologies, LTC --- IBM Deutschland Research Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: Fix client crash on connection close
On 03/02/2012 01:49 PM, Jiri Denemark wrote:
A multi-threaded client with event loop may crash if one of its threads
closes a connection while event loop is in the middle of sending
keep-alive message (either request or response). The right place for it
is inside virNetClientIOEventLoop() between poll() and
virNetClientLock(). We should only close a connection directly if no-one
is using it and defer the closing to the last user otherwise. So far we
only did so if the close was initiated by keep-alive timeout.
---
src/rpc/virnetclient.c | 18 --
1 files changed, 4 insertions(+), 14 deletions(-)
@@ -512,19 +510,11 @@ virNetClientCloseLocked(virNetClientPtr client)
void virNetClientClose(virNetClientPtr client)
{
-if (!client)
-return;
-
-virNetClientLock(client);
-virNetClientCloseLocked(client);
-virNetClientUnlock(client);
-}
-
-static void
-virNetClientRequestClose(virNetClientPtr client)
-{
VIR_DEBUG(client=%p, client);
The diff that git picked is a bit confusing; but it looks like all you
are doing is stating that virNetClientClose should do the same thing as
virNetClientRequestClose did (which is safer); and now that they do the
same, you don't need two names, so pick the shorter name.
ACK.
--
Eric Blake ebl...@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 0/4] Support mac and port profile for interface type='hostdev'
On 3/2/12 12:54 PM, Laine Stump la...@laine.org wrote: On 03/02/2012 03:03 PM, Roopa Prabhu wrote: On 3/2/12 11:04 AM, Laine Stump la...@laine.org wrote: On 03/01/2012 04:02 AM, Roopa Prabhu wrote: This patch series is based on laines patches to support interface type='hostdev'. https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html It support to set mac and port profile on an interface of type hostdev. * If virtualport is specified, the existing virtual port functions are called to set mac, vlan and port profile. I'm unable to test that part, as I don't have any 802.1QbX capable switches (and it sounds like the design is problematic anyway.) The design is still fine for 802.1Qbh. Yes, my apologies for misinterpreting all the exchanges. No problem atall. I have tested it. 802.1Qbh does not need a macvtap device. For 802.1Qbg in v2 (was planning on pushing it the next hr), I'll try to review it as quickly as possible. I have put a check for 802.1Qbg and hostdevs and fail as suggested by stefan. I'm quickly learning that I understood much less about 802.1QbX (and in particular, how it's been implemented) than I thought! (Fortunately, as a result of all this, I think I now understand it a bit better). And I am understanding 802.1Qbg a bit better now :) * If virtualport is not specified and device is a sriov virtual function, - mac is set using IFLA_VF_MAC Success!! I tried this for VFs that have a netdev driver attached, and VFs that don't, and it behaved properly in both cases - when the guest is started, the MAC address is set properly for the guest to use, and when the guest is stopped, the MAC address of that VF is restored to its original value (implying that your code to save the old MAC address works properly). Nice. Thanks for trying it out. * If virtualport is not specified and device is a non-sriov virtual function, - mac is set using existing SIOCGIFHWADDR (This requires that the netdev be present on the host before starting the VM) This one has a problem, at least with my non-sriov hardware (which happens to be the onboard NetXtreme device of a Thinkstation, using the tg3 driver) it appears the MAC address gets reset to its original setting at some point after libvirt changes it. To help understand what happens - assume the device's original MAC address is o:o:o:o:o:o, and my xml looks like this: interface type='hostdev' managed='yes' mac address='n:n:n:n:n:n'/ ... /interface When the guest boots up, ifconfig shows there is an interface with mac address o:o:o:o:o:o. Additionally, if I manually change the mac address to p:p:p:p:p:p on the host before starting the guest, when the guest boots, ifconfig shows the mac address as... o:o:o:o:o:o. So, whether or not libvirt is successfully setting the mac address, it's getting reset (probably by the card's firmware). Yes this I kind of expected. It depends on the driver. I thought there are some drivers that remember the mac address set by SIOCGIFHWADDR. But my assumption was totally based on the fact that we wanted to add support for all devices. Having the code there just means we try to set the device mac. It takes effect only if the hw supports it. If there was just a way to determine that at runtime, but it seems that by the time the MAC address has been reset, we are no longer able to call the ioctl to check the address :-( So perhaps this is another case of wanting to do something that just isn't possible, and the way out is to simply generate an error on domain startup if the netdev being passed through isn't a VF? We can do this too. Only support it for sriov vf's. Yes, if it's going to silently do the wrong thing, maybe we should leave the SIOCGIFHWADDR code in there for reference, but also add a failure condition if the card isn't SRIOV. Ok. Heres what I will do (If I understand you correctly): - I will call the mac/portprofile set functions for sriov devices only. - Throw an error for non-sriov devices - Keep the mac set code for non-sriov devices around for reference (I guess this means my effort to make sure USB ethernets were also supported was kind of pointless, since they're sure to have the same problems :-P Mostly I only included that support to promote code sharing and consistency, though, so I'm not really disappointed.) :) Thanks Laine. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] virsh: add option aliases
In the past, we have created some virsh options with less-than-stellar
names. For back-compat reasons, those names must continue to parse,
but we don't want to document them in help output. This introduces
a new option type, an alias, which points to a canonical option name
later in the option list.
I'm actually quite impressed that our code has already been factored
to do all option parsing through common entry points, such that I
got this added in relatively few lines of code!
* tools/virsh.c (VSH_OT_ALIAS): New option type.
(opts_echo): Hook up an alias, for easy testing.
(vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for
aliases.
* tests/virshtest.c (mymain): Test new feature.
---
tests/virshtest.c |6 ++
tools/virsh.c | 28 ++--
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 6474c19..87b1336 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -386,6 +386,12 @@ mymain(void)
DO_TEST(30, --shell a\n,
echo \t '-'\-\ \t --shell \t a);
+/* Tests of alias handling. */
+DO_TEST(31, hello\n, echo, --string, hello);
+DO_TEST(32, hello\n, echo --string hello);
+DO_TEST(33, hello\n, echo, --str, hello);
+DO_TEST(34, hello\n, echo --str hello);
+
# undef DO_TEST
VIR_FREE(custom_uri);
diff --git a/tools/virsh.c b/tools/virsh.c
index aef050f..77cf4ac 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -138,7 +138,8 @@ typedef enum {
VSH_OT_STRING, /* optional string option */
VSH_OT_INT, /* optional or mandatory int option */
VSH_OT_DATA, /* string data (as non-option) */
-VSH_OT_ARGV /* remaining arguments */
+VSH_OT_ARGV, /* remaining arguments */
+VSH_OT_ALIAS,/* alternate spelling for a later argument */
} vshCmdOptType;
/*
@@ -190,7 +191,8 @@ typedef struct {
const char *name; /* the name of option, or NULL for list end */
vshCmdOptType type; /* option type */
unsigned int flags; /* flags */
-const char *help; /* non-NULL help string */
+const char *help; /* non-NULL help string; or for VSH_OT_ALIAS
+ * the name of a later public option */
} vshCmdOptDef;
/*
@@ -15209,6 +15211,7 @@ static const vshCmdInfo info_echo[] = {
static const vshCmdOptDef opts_echo[] = {
{shell, VSH_OT_BOOL, 0, N_(escape for shell use)},
{xml, VSH_OT_BOOL, 0, N_(escape for XML use)},
+{str, VSH_OT_ALIAS, 0, string},
{string, VSH_OT_ARGV, 0, N_(arguments to echo)},
{NULL, 0, 0, NULL}
};
@@ -17256,6 +17259,18 @@ vshCmddefOptParse(const vshCmdDef *cmd, uint32_t
*opts_need_arg,
return -1; /* bool options can't be mandatory */
continue;
}
+if (opt-type == VSH_OT_ALIAS) {
+int j;
+if (opt-flags || !opt-help)
+return -1; /* alias options are tracked by the original name */
+for (j = i + 1; cmd-opts[j].name; j++) {
+if (STREQ(opt-help, cmd-opts[j].name))
+break;
+}
+if (!cmd-opts[j].name)
+return -1; /* alias option must map to a later option name */
+continue;
+}
if (opt-flags VSH_OFLAG_REQ_OPT) {
if (opt-flags VSH_OFLAG_REQ)
*opts_required |= 1 i;
@@ -17287,6 +17302,10 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef
*cmd, const char *name,
const vshCmdOptDef *opt = cmd-opts[i];
if (STREQ(opt-name, name)) {
+if (opt-type == VSH_OT_ALIAS) {
+name = opt-help;
+continue;
+}
if ((*opts_seen (1 i)) opt-type != VSH_OT_ARGV) {
vshError(ctl, _(option --%s already seen), name);
return NULL;
@@ -17465,6 +17484,9 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
: _([%s]...);
}
break;
+case VSH_OT_ALIAS:
+/* aliases are intentionally undocumented */
+continue;
default:
assert(0);
}
@@ -17506,6 +17528,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
shortopt ? _([--%s] string) : _(%s),
opt-name);
break;
+case VSH_OT_ALIAS:
+continue;
default:
assert(0);
}
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] virsh: add command and option aliases
I'm tired of publicizing typos. These patches preserve full back-compat (scripts that used the old spelling will continue to work), we just don't document the old spellings. I started this patch because we have the memory commands that take a --kilobyte argument when they really mean KiB (kibibyte or 1024); I plan to use the aliasing feature in my later units cleanup series to fix this naming to instead take scaled sizes (such as --size=1M or --size=1024bytes); at which point --kilobyte will be the undocumented alias so that we no longer document a name that might then be confused for 1000. Eric Blake (3): virsh: add option aliases virsh: use option aliases virsh: add command aliases, and rename nodedev-detach tests/virshtest.c |6 +++ tools/virsh.c | 107 +--- tools/virsh.pod | 28 +++-- 3 files changed, 97 insertions(+), 44 deletions(-) -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] virsh: use option aliases
Command line interfaces should use dash, not underscore, as many
keyboard layouts allow that to be typed with fewer shift key presses.
Also, the US spelling of --tunneled gets more google hits than the
UK spelling of --tunnelled.
* tools/virsh.c (opts_migrate): Allow US variant.
(opts_blkdeviotune): Prefer - over _.
* tools/virsh.pod (blkdeviotune): Fix spelling.
---
tools/virsh.c | 49 +++--
tools/virsh.pod | 18 +-
2 files changed, 40 insertions(+), 27 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 77cf4ac..75a1a3b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -6879,6 +6879,7 @@ static const vshCmdOptDef opts_migrate[] = {
{live, VSH_OT_BOOL, 0, N_(live migration)},
{p2p, VSH_OT_BOOL, 0, N_(peer-2-peer migration)},
{direct, VSH_OT_BOOL, 0, N_(direct migration)},
+{tunneled, VSH_OT_ALIAS, 0, tunnelled},
{tunnelled, VSH_OT_BOOL, 0, N_(tunnelled migration)},
{persistent, VSH_OT_BOOL, 0, N_(persist VM on destination)},
{undefinesource, VSH_OT_BOOL, 0, N_(undefine VM on source)},
@@ -7574,17 +7575,23 @@ static const vshCmdInfo info_blkdeviotune[] = {
static const vshCmdOptDef opts_blkdeviotune[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)},
-{total_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE,
+{total_bytes_sec, VSH_OT_ALIAS, 0, total-bytes-sec},
+{total-bytes-sec, VSH_OT_INT, VSH_OFLAG_NONE,
N_(total throughput limit in bytes per second)},
-{read_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE,
+{read_bytes_sec, VSH_OT_ALIAS, 0, read-bytes-sec},
+{read-bytes-sec, VSH_OT_INT, VSH_OFLAG_NONE,
N_(read throughput limit in bytes per second)},
-{write_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE,
+{write_bytes_sec, VSH_OT_ALIAS, 0, write-bytes-sec},
+{write-bytes-sec, VSH_OT_INT, VSH_OFLAG_NONE,
N_(write throughput limit in bytes per second)},
-{total_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE,
+{total_iops_sec, VSH_OT_ALIAS, 0, total-iops-sec},
+{total-iops-sec, VSH_OT_INT, VSH_OFLAG_NONE,
N_(total I/O operations limit per second)},
-{read_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE,
+{read_iops_sec, VSH_OT_ALIAS, 0, read-iops-sec},
+{read-iops-sec, VSH_OT_INT, VSH_OFLAG_NONE,
N_(read I/O operations limit per second)},
-{write_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE,
+{write_iops_sec, VSH_OT_ALIAS, 0, write-iops-sec},
+{write-iops-sec, VSH_OT_INT, VSH_OFLAG_NONE,
N_(write I/O operations limit per second)},
{config, VSH_OT_BOOL, 0, N_(affect next boot)},
{live, VSH_OT_BOOL, 0, N_(affect running domain)},
@@ -7630,7 +7637,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptString(cmd, device, disk) 0)
goto cleanup;
-if ((rv = vshCommandOptULongLong(cmd, total_bytes_sec,
total_bytes_sec)) 0) {
+if ((rv = vshCommandOptULongLong(cmd, total-bytes-sec,
+ total_bytes_sec)) 0) {
vshError(ctl, %s,
_(Unable to parse integer parameter));
goto cleanup;
@@ -7638,7 +7646,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
nparams++;
}
-if ((rv = vshCommandOptULongLong(cmd, read_bytes_sec, read_bytes_sec))
0) {
+if ((rv = vshCommandOptULongLong(cmd, read-bytes-sec,
+ read_bytes_sec)) 0) {
vshError(ctl, %s,
_(Unable to parse integer parameter));
goto cleanup;
@@ -7646,7 +7655,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
nparams++;
}
-if ((rv = vshCommandOptULongLong(cmd, write_bytes_sec,
write_bytes_sec)) 0) {
+if ((rv = vshCommandOptULongLong(cmd, write-bytes-sec,
+ write_bytes_sec)) 0) {
vshError(ctl, %s,
_(Unable to parse integer parameter));
goto cleanup;
@@ -7654,7 +7664,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
nparams++;
}
-if ((rv = vshCommandOptULongLong(cmd, total_iops_sec, total_iops_sec))
0) {
+if ((rv = vshCommandOptULongLong(cmd, total-iops-sec,
+ total_iops_sec)) 0) {
vshError(ctl, %s,
_(Unable to parse integer parameter));
goto cleanup;
@@ -7662,7 +7673,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
nparams++;
}
-if ((rv = vshCommandOptULongLong(cmd, read_iops_sec, read_iops_sec))
0) {
+if ((rv = vshCommandOptULongLong(cmd, read-iops-sec,
+ read_iops_sec)) 0) {
vshError(ctl, %s,
_(Unable to parse integer parameter));
goto cleanup;
@@ -7670,7 +7682,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
nparams++;
}
-if ((rv = vshCommandOptULongLong(cmd,
[libvirt] [PATCH 3/3] virsh: add command aliases, and rename nodedev-detach
Just because our public API has a typo doesn't mean that virsh
has to keep the typo.
* tools/virsh.c (VSH_CMD_FLAG_ALIAS): New flag.
(nodedevCmds): Use it.
(cmdHelp): Omit alias commands.
(cmdNodeDeviceDettach): Rename...
(cmdNodeDeviceDetach): ...to this.
* tools/virsh.pod (nodedev-detach): Document it.
---
tools/virsh.c | 30 +++---
tools/virsh.pod | 10 ++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 75a1a3b..4361a6b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -212,6 +212,7 @@ typedef struct vshCmdOpt {
*/
enum {
VSH_CMD_FLAG_NOCONNECT = (1 0), /* no prior connection needed */
+VSH_CMD_FLAG_ALIAS = (1 1), /* command is an alias */
};
/*
@@ -685,9 +686,12 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, _( %s (help keyword '%s'):\n), grp-name,
grp-keyword);
-for (def = grp-commands; def-name; def++)
+for (def = grp-commands; def-name; def++) {
+if (def-flags VSH_CMD_FLAG_ALIAS)
+continue;
vshPrint(ctl, %-30s %s\n, def-name,
_(vshCmddefGetInfo(def, help)));
+}
vshPrint(ctl, \n);
}
@@ -12939,22 +12943,22 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd
*cmd)
}
/*
- * nodedev-dettach command
+ * nodedev-detach command
*/
-static const vshCmdInfo info_node_device_dettach[] = {
-{help, N_(dettach node device from its device driver)},
-{desc, N_(Dettach node device from its device driver before assigning
to a domain.)},
+static const vshCmdInfo info_node_device_detach[] = {
+{help, N_(detach node device from its device driver)},
+{desc, N_(Detach node device from its device driver before assigning to
a domain.)},
{NULL, NULL}
};
-static const vshCmdOptDef opts_node_device_dettach[] = {
+static const vshCmdOptDef opts_node_device_detach[] = {
{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(device key)},
{NULL, 0, 0, NULL}
};
static bool
-cmdNodeDeviceDettach (vshControl *ctl, const vshCmd *cmd)
+cmdNodeDeviceDetach (vshControl *ctl, const vshCmd *cmd)
{
const char *name = NULL;
virNodeDevicePtr device;
@@ -12969,10 +12973,12 @@ cmdNodeDeviceDettach (vshControl *ctl, const vshCmd
*cmd)
return false;
}
+/* Yes, our public API is misspelled. At least virsh can accept
+ * either spelling. */
if (virNodeDeviceDettach(device) == 0) {
-vshPrint(ctl, _(Device %s dettached\n), name);
+vshPrint(ctl, _(Device %s detached\n), name);
} else {
-vshError(ctl, _(Failed to dettach device %s), name);
+vshError(ctl, _(Failed to detach device %s), name);
ret = false;
}
virNodeDeviceFree(device);
@@ -17090,8 +17096,10 @@ static const vshCmdDef nodedevCmds[] = {
info_node_device_create, 0},
{nodedev-destroy, cmdNodeDeviceDestroy, opts_node_device_destroy,
info_node_device_destroy, 0},
-{nodedev-dettach, cmdNodeDeviceDettach, opts_node_device_dettach,
- info_node_device_dettach, 0},
+{nodedev-detach, cmdNodeDeviceDetach, opts_node_device_detach,
+ info_node_device_detach, 0},
+{nodedev-dettach, cmdNodeDeviceDetach, opts_node_device_detach,
+ info_node_device_detach, VSH_CMD_FLAG_ALIAS},
{nodedev-dumpxml, cmdNodeDeviceDumpXML, opts_node_device_dumpxml,
info_node_device_dumpxml, 0},
{nodedev-list, cmdNodeListDevices, opts_node_list_devices,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1bc55c4..b365624 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1442,7 +1442,7 @@ Attach a device to the domain, using a device definition
in an XML file.
See the documentation to learn about libvirt XML format for a device.
For cdrom and floppy devices, this command only replaces the media within
the single existing device; consider using Bupdate-device for this usage.
-For passthrough host devices, see also Bnodedev-dettach, needed if
+For passthrough host devices, see also Bnodedev-detach, needed if
the device does not use managed mode.
=item Battach-disk Idomain-id Isource Itarget
@@ -1571,7 +1571,7 @@ guest domains, nor by multiple active guests at once. If
the
hostdev description includes the attribute Bmanaged='yes', and the
hypervisor driver supports it, then the device is in managed mode, and
attempts to use that passthrough device in an active guest will
-automatically behave as if Bnodedev-dettach (guest start, device
+automatically behave as if Bnodedev-detach (guest start, device
hot-plug) and Bnodedev-reattach (guest stop, device hot-unplug) were
called at the right points (currently, qemu does this for PCI devices,
but not USB). If a device is not marked as managed, then it must
@@ -1596,11 +1596,13 @@ Destroy (stop) a device on the host. Note that this
makes libvirt
quit managing a host device, and
Re: [libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
On 03/01/2012 11:56 AM, Michal Privoznik wrote:
Some nits are generated during XML parse (e.g. MAC address of
This reads awkwardly; how about:
s/nits/members/
an interface); However, with current implementation, if we
are plugging a device both to persistent and live config,
we parse given XML twice: first time for live, second for config.
This is wrong then as the second time we are not guaranteed
to generate same values as we did for the first time.
To prevent that we need to create a copy of DeviceDefPtr;
This is done through format/parse process instead of writing
functions for deep copy as it is easier to maintain:
adding new field to any virDomain*DefPtr doesn't require change
of copying function.
---
src/conf/domain_conf.c | 94
++
src/conf/domain_conf.h |3 +
src/libvirt_private.syms |1 +
src/qemu/qemu_driver.c | 40 +++
4 files changed, 121 insertions(+), 17 deletions(-)
+
+#define VIR_DOMAIN_DEVICE_FORMAT(func) \
+if (func 0) \
+goto cleanup; \
+xmlStr = virBufferContentAndReset(buf); \
+ret = virDomainDeviceDefParse(caps, def, xmlStr, flags)
If you sink these two lines to occur after the switch statement closes,
then you can avoid this macro. That is...
+
+virDomainDeviceDefPtr
+virDomainDeviceDefCopy(virCapsPtr caps,
+ const virDomainDefPtr def,
+ virDomainDeviceDefPtr src)
+{
+virDomainDeviceDefPtr ret = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int flags = VIR_DOMAIN_XML_INACTIVE;
+char *xmlStr = NULL;
int rc;
+
+switch(src-type) {
+case VIR_DOMAIN_DEVICE_DISK:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainDiskDefFormat(buf,
+src-data.disk,
+flags));
case VIR_DOMAIN_DEVICE_DISK:
rc = virDomainDiskDefFormat(buf, src-data.disk, flags);
break;
...
+default:
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Copying definition of '%d' type
+ is not implemented yet.),
+ src-type);
goto cleanup;
+break;
+}
if (fc 0)
goto cleanup;
xmlStr = virBufferContentAndReset(buf);
ret = virDomainDeviceDefParse(caps, def, xmlStr, flags);
+
+cleanup:
+VIR_FREE(xmlStr);
+return ret;
+}
@@ -5694,6 +5698,8 @@ endjob:
cleanup:
virDomainDefFree(vmdef);
virDomainDeviceDefFree(dev);
+if (free_dev_copy)
+virDomainDeviceDefFree(dev_copy);
You could also avoid free_dev_copy, and just say if (dev != dev_copy) here.
Overall, this looks like a reasonable patch. But I suggested enough
cleanups that it's probably worth seeing a v2.
--
Eric Blake ebl...@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 v2] libvirt-guests: Add parallel startup and shutdown of guests
On 03/01/2012 07:23 AM, Peter Krempa wrote:
With this patch, it's possible to shut down guests in parallel. Parallel
startup was possible before, but this functionality was not documented
properly.
To enable parallel startup set the START_DELAY to 0.
Parallel shutdown has a configurable parameter PARALLEL_SHUTDOWN that
defines the number of machines being shut down in parallel. Enabling
this feature changes the semantics of SHUTDOWN_TIMEOUT parameter that is
applied as a cumulative timeout to shutdown all guests on a URI.
---
+# shutdown_guests_parallel URI GUESTS
+# Shutdown guests GUESTS on machine URI in parallel
+shutdown_guests_parallel()
+{
+uri=$1
+guests=$2
+
+on_shutdown=
check_timeout=false
+timeout=$SHUTDOWN_TIMEOUT
if [ $timeout -gt 0 ]; then
check_timeout=true
fi
+while [ -n $on_shutdown ] || [ -n $guests ]; do
+while [ -n $guests ]
+ [ $(guest_count $on_shutdown) -lt $PARALLEL_SHUTDOWN ]; do
+set -- $guests
+guest=$1
+shift
+guests=$*
+shutdown_guest_async $uri $guest
+on_shutdown=$on_shutdown $guest
+done
+sleep 1
if $check_timeout; then
+timeout=$(($timeout - 1))
+if [ $timeout -le 0 ]; then
+eval_gettext Timeout expired while shutting down domains; echo
+RETVAL=1
+return
+fi
fi
+on_shutdown_prev=$on_shutdown
+on_shutdown=$(check_guests_shutdown $uri $on_shutdown)
+print_guests_shutdown $uri $on_shutdown_prev $on_shutdown
+done
+}
+
-# number of seconds we're willing to wait for a guest to shut down
+# If set to non-zero, shutdown will suspend guests concurrently. Number of
+# guests on shutdown at any time will not exceed number set in this variable.
+#PARALLEL_SHUTDOWN=0
+
+# Number of seconds we're willing to wait for a guest to shut down. If
parallel
+# shutdown is enabled, this timeout applies as a timeout for shutting down
all
+# guests on a single URI defined in the variable URIS. This must be set to
+# a nonzero positive value if the shutdown action is requested.
Change the last sentence:
If this is 0, then there is no time out (use with caution, as guests
might not respond to a shutdown request). (Hmm, maybe we want to
default to 300 [5 minutes], and document our non-zero default, so that
you have to explicitly request 0 to avoid timeouts.)
ACK with those lines added to shutdown_guests_parallel, and the wording
change to the config file, and with the optional change to the timeout
default.
--
Eric Blake ebl...@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] cpu: Add new flag supported by qemu to the cpu definition
On 02/29/2012 07:53 AM, Peter Krempa wrote:
Some new cpu features were added to qemu. This patch adds some of them
to our CPU map.
---
to ease review, here's an excerpt from qemu.git/target-i386/cpuid.c to ease
the review:
static const char *ext3_feature_name[] = {
lahf_lm /* AMD LahfSahf */, cmp_legacy, svm, extapic /* AMD
ExtApicSpace */,
cr8legacy /* AMD AltMovCr8 */, abm, sse4a, misalignsse,
3dnowprefetch, osvw, ibs, xop,
skinit, wdt, NULL, NULL,
fma4, NULL, cvt16, nodeid_msr,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
};
Thanks; that helped review. ACK.
--
Eric Blake ebl...@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 01/17] conf: add missing device types to virDomainDevice(Type|Def)
On 02/28/2012 01:14 PM, Laine Stump wrote: Not all device types were represented in virDomainDeviceType, so some types of devices couldn't be represented in a virDomainDeviceDef (which requires a different type of pointer in the union for each different kind of device). Since serial, parallel, channel, and console devices are all virDomainChrDef, and the virDomainDeviceType is never used to produce a string from the type (and only used in the other direction internally to code, never to produce XML), I only added one CHR type, which is associated with virDomainChrDefPtr chr in the union. --- New patch in V2. src/conf/domain_conf.c |6 +- src/conf/domain_conf.h | 11 +-- 2 files changed, 14 insertions(+), 3 deletions(-) ACK. -- Eric Blake ebl...@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 02/17] conf: relocate virDomainDeviceDef and virDomainHostdevDef
On 02/28/2012 01:14 PM, Laine Stump wrote: This patch is only code movement + adding some forward definitions of typedefs. virDomainHostdevDef (not just a pointer to it, but an actual object) will be needed in virDomainNetDef and virDomainActualNetDef, so it must be relocated earlier in the file. Likewise, virDomainDeviceDef will be needed in virDomainHostdevDef, so it must be moved up even earlier. This, in turn, creates a forward reference problem, but fortunately only with pointers to other device types, so their typedefs can be moved up in the file, eliminating the problem. --- V2: aside from the fact that there are now more types of device to have their definitions/enums relocated, I learned that older versions of gcc will not allow a duplicate typedef, even if it is identical to the original, so rather than just copying the device typedefs from their original locations just above each struct definition, I had to move them, removing the original. src/conf/domain_conf.h | 262 ++-- 1 files changed, 141 insertions(+), 121 deletions(-) ACK. -- Eric Blake ebl...@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 03/17] conf: reorder static functions in domain_conf.c
On 02/28/2012 01:14 PM, Laine Stump wrote: No code change, movement only. This is necessary to eliminate forward references. --- V2: No change src/conf/domain_conf.c | 417 1 files changed, 208 insertions(+), 209 deletions(-) ACK. -- Eric Blake ebl...@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 04/17] qemu: rename virDomainDeviceInfoPtr variables to avoid confusion
On 02/28/2012 01:14 PM, Laine Stump wrote: The virDomainDeviceInfoPtrs in qemuCollectPCIAddress and qemuComparePCIDevice are named dev and dev1, but those functions will be changed (in order to match a change in the args sent to virDomainDeviceInfoIterate() callback args) to contain a virDomainDeviceDefPtr device. This patch renames dev to info (and dev[n] to info[n]) to avoid later confusion. --- new Patch in V2. ACK. -- Eric Blake ebl...@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 05/17] conf: add device pointer to args of virDomainDeviceInfoIterate callback
On 02/28/2012 01:14 PM, Laine Stump wrote: There will be cases where the iterator callback will need to know the type of the device whose info is being operated on, and possibly even need to use some of the device's config. This patch adds a virDomainDeviceDefPtr to the args of every callback, and fills it in appropriately as the devices are iterated through. --- New patch in V2. ACK. -- Eric Blake ebl...@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 06/17] conf: make hostdev info a separate object
On 02/28/2012 01:14 PM, Laine Stump wrote:
In order to allow for a virDomainHostdevDef that uses the
virDomainDeviceInfo of a higher level device (such as a
virDomainNetDef), this patch changes the virDomainDeviceInfo in the
HostdevDef into a virDomainDeviceInfoPtr. Rather than adding checks
all over the code to check for a null info, we just guarantee that it
is always valid. The new function virDomainHostdevDefAlloc() allocates
a virDomainDeviceInfo and plugs it in, and virDomainHostdevDefFree()
makes sure it is freed.
There were 4 places allocating virDomainHostdevDefs, all of them
parsers of one sort or another, and those have all had their
VIR_ALLOC(hostdev) changed to virDomainHostdevDefAlloc(). Other than
that, and the new functions, all the rest of the changes are just
mechanical removals of or changing . to -.
---
V2: also add a virDomainDeviceInfoClear() function.
@@ -6653,43 +6653,37 @@ cleanup:
static virDomainHostdevDefPtr
qemuParseCommandLinePCI(const char *val)
{
-
-cleanup:
return def;
+
+ error:
Unusual spacing on the label.
@@ -6768,9 +6757,11 @@ qemuParseCommandLineUSB(const char *val)
def-source.subsys.u.usb.vendor = first;
def-source.subsys.u.usb.product = second;
}
-
-cleanup:
return def;
+
+ error:
And again.
ACK.
--
Eric Blake ebl...@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 07/17] conf: HostdevDef parse/format helper functions
On 02/28/2012 01:14 PM, Laine Stump wrote: In an upcoming patch, virDomainNetDef will acquire a virDomainHostdevDef, and the interface XML will take on some of the elements of a hostdev. To avoid duplicating the code for parsing and formatting the source element (which will be nearly identical in these two cases), this patch factors those parts out of the HostdevDef's parse and format functions, and puts them into separate helper functions that are now called by the HostdevDef parser/formatter, and will soon be called by the NetDef parser/formatter. One change in behavior - previously virDomainHostdevDefParseXML() had diverged from current common coding practice by logging an error and failing if it found any subelements of hostdev other than those it understood (standard libvirt practice is to ignore/discard unknown elements and attributes during parse). The new helper function ignores unknown elements, and thus so does the new virDomainHostdevDefParseXML. --- V2: Unchanged from V1. I'll take your word that it's unchanged, so ACK. -- Eric Blake ebl...@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 08/17] conf: give each hostdevdef a parent pointer
On 02/28/2012 01:14 PM, Laine Stump wrote: The parent can be any type of device. It defaults to type=none, and a NULL pointer. The intent is that if a hostdevdef is contained in the def for a higher level device (e.g. virDomainNetDef), hostdev-parent will point to the higher level device, and type will be set to that type of device. This way, during attach and detach of the device, parent can be checked, and appropriate callouts made to do higher level device initialization (e.g. setting MAC address). Also, although these hostdevs with parents will be added to a domain's hostdevs list, they will be treated slightly differently when traversing the list, e.g. virDomainHostdefDefFree for a hostdev that has a parent doesn't need to be called (and will be a NOP); it will simply be removed from the list (since the parent device object is in its own type-specific list, and will be freed from there). --- V2: unchanged from V1 src/conf/domain_conf.c | 12 ++-- src/conf/domain_conf.h |1 + 2 files changed, 11 insertions(+), 2 deletions(-) ACK. -- Eric Blake ebl...@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] Correct a check for capacity arg of storageVolumeResize()
From: Zeeshan Ali (Khattak) zeesha...@gnome.org
Lets say I got a volume with '1G' allocation and '10G' capacity. The
available space in the parent pool is '5G'. With the current check for
overcapacity, I can only try to resize to = '6G'. You see the problem?
---
src/storage/storage_driver.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9130a40..66811ce 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1758,8 +1758,8 @@ storageVolumeResize(virStorageVolPtr obj,
goto out;
}
-if (abs_capacity vol-allocation + pool-def-available) {
-virStorageReportError(VIR_ERR_INVALID_ARG,
+if (abs_capacity vol-capacity + pool-def-available) {
+virStorageReportError(VIR_ERR_OPERATION_FAILED,
_(Not enough space left on storage pool));
goto out;
}
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Correct a check for capacity arg of storageVolumeResize()
On 03/02/2012 08:18 PM, Zeeshan Ali (Khattak) wrote:
From: Zeeshan Ali (Khattak) zeesha...@gnome.org
Lets say I got a volume with '1G' allocation and '10G' capacity. The
available space in the parent pool is '5G'. With the current check for
overcapacity, I can only try to resize to = '6G'. You see the problem?
---
src/storage/storage_driver.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9130a40..66811ce 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1758,8 +1758,8 @@ storageVolumeResize(virStorageVolPtr obj,
goto out;
}
-if (abs_capacity vol-allocation + pool-def-available) {
-virStorageReportError(VIR_ERR_INVALID_ARG,
+if (abs_capacity vol-capacity + pool-def-available) {
+virStorageReportError(VIR_ERR_OPERATION_FAILED,
ACK and pushed.
--
Eric Blake ebl...@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 09/17] conf: put subsys part of virDomainHostdevDef into its own struct
On 02/28/2012 01:14 PM, Laine Stump wrote: To shorten some new code that accesses the many fields within the subsys struct of a hostdev, create a separate toplevel, typedefed virDomainHostdevSubsys struct so that we can define temporary pointers to the subsys part. --- New patch for V2. src/conf/domain_conf.h | 31 ++- 1 files changed, 18 insertions(+), 13 deletions(-) ACK. -- Eric Blake ebl...@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 11/17] qemu: re-order functions in qemu_hotplug.c
On 02/28/2012 01:14 PM, Laine Stump wrote: Code movement only, no functional change. This is necessary to prevent a forward reference in an upcoming patch. --- New patch for V2. src/qemu/qemu_hotplug.c | 289 --- 1 files changed, 145 insertions(+), 144 deletions(-) ACK. -- Eric Blake ebl...@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 10/17] conf: hostdev utility functions
On 02/28/2012 01:14 PM, Laine Stump wrote:
Three new functions useful in other files:
virDomainHostdevInsert:
Add a new hostdev at the end of the array. This would more sensibly be
called virDomainHostdevAppend, but the existing functions for other
types of devices are called Insert.
virDomainHostdevRemove:
Eliminates one entry from the hostdevs array, but doesn't free it;
patterned after the code at the end of the two
qemuDomainDetachHostXXXDevice functions (and also other pre-existing
virDomainXXXRemove functions for other device types).
virDomainHostdevFind:
This function is patterned from the search loops at the top of
qemuDomainDetachHostPciDevice and qemuDomainDetachHostUsbDevice, and
will be used to re-factor those (and other detach-related) functions.
---
New patch for V2.
src/conf/domain_conf.c | 94
++
src/conf/domain_conf.h |5 ++
src/libvirt_private.syms |3 +
3 files changed, 102 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93fd8d7..94ee634 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6769,6 +6769,100 @@ virDomainChrTargetTypeToString(int deviceType,
}
int
+virDomainHostdevInsert(virDomainDefPtr def, virDomainHostdevDefPtr hostdev)
+{
+if (VIR_REALLOC_N(def-hostdevs, def-nhostdevs + 1) 0)
+return -1;
+def-hostdevs[def-nhostdevs++] = hostdev;
Is the double space intended?
+return 0;
+}
+
+void
+virDomainHostdevRemove(virDomainDefPtr def, size_t i)
+{
+if (def-nhostdevs 1) {
+memmove(def-hostdevs + i,
+def-hostdevs + i + 1,
+sizeof(*def-hostdevs) *
+(def-nhostdevs - (i + 1)));
+def-nhostdevs--;
+if (VIR_REALLOC_N(def-hostdevs, def-nhostdevs) 0) {
I know this is copy and paste, but we could clean this pattern up
throughout the file (later) to use VIR_SHRINK_N.
ACK.
--
Eric Blake ebl...@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
