[libvirt] [PATCH python] Fix calling of virStreamSend method
Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper. Reported-by: Robie Basak <robie.basak@canonical.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py index cd44314..ce82da6 100644 --- a/libvirt-override-virStream.py +++ b/libvirt-override-virStream.py @@ -122,6 +122,6 @@ with the call, but may instead be delayed until a subsequent call. """ - ret = libvirtmod.virStreamSend(self._o, data, len(data)) + ret = libvirtmod.virStreamSend(self._o, data) if ret == -1: raise libvirtError ('virStreamSend() failed') return ret -- 1.8.4.2
On 23/01/14 22:25, Daniel P. Berrange wrote: About the subject prefix, [libvirt-python] should be better, like what we did for Perl binding.
Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper.
Reported-by: Robie Basak <robie.basak@canonical.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py index cd44314..ce82da6 100644 --- a/libvirt-override-virStream.py +++ b/libvirt-override-virStream.py @@ -122,6 +122,6 @@ with the call, but may instead be delayed until a subsequent call. """ - ret = libvirtmod.virStreamSend(self._o, data, len(data)) + ret = libvirtmod.virStreamSend(self._o, data) if ret == -1: raise libvirtError ('virStreamSend() failed') return ret ACK
On 01/23/2014 07:47 AM, Osier Yang wrote:
On 23/01/14 22:25, Daniel P. Berrange wrote:
About the subject prefix, [libvirt-python] should be better, like what we did for Perl binding.
Not possible. Anything sent to the list gets '[libvirt]' prepended, and then 'git format-patch --subject-prefix="PATCH python"' prepends [PATCH python]. Your idea of setting libvirt-python in the git portion of the prefix would be redundant with the libvirt in the list portion of the prefix. I'm fine with the approach Daniel used. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/23/2014 07:25 AM, Daniel P. Berrange wrote:
Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper.
Reported-by: Robie Basak <robie.basak@canonical.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Hmm, you pushed this patch onto the release before 1.2.1; I had to push a merge commit to get the master branch to include both this and the release. I'm not sure why git let you push a non-fast-forward, unless DV forgot to push the 1.2.1 tag to the master branch until after you had already pushed this. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Feb 03, 2014 at 02:32:27PM -0700, Eric Blake wrote:
On 01/23/2014 07:25 AM, Daniel P. Berrange wrote:
Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper.
Reported-by: Robie Basak <robie.basak@canonical.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Hmm, you pushed this patch onto the release before 1.2.1; I had to push a merge commit to get the master branch to include both this and the release. I'm not sure why git let you push a non-fast-forward, unless DV forgot to push the 1.2.1 tag to the master branch until after you had already pushed this.
That is very odd - the commit dates all show a normal sequence of committing. I can only imagine that perhaps we forgot to set the "deny merge" bit in the git repo config and I wasn't up2date when I pushed ? The git config looks OK right now, but I see you modified it last night, so did you fix something ? 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 :|
On 02/04/2014 03:21 AM, Daniel P. Berrange wrote:
On Mon, Feb 03, 2014 at 02:32:27PM -0700, Eric Blake wrote:
On 01/23/2014 07:25 AM, Daniel P. Berrange wrote:
Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper.
Reported-by: Robie Basak <robie.basak@canonical.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Hmm, you pushed this patch onto the release before 1.2.1; I had to push a merge commit to get the master branch to include both this and the release. I'm not sure why git let you push a non-fast-forward, unless DV forgot to push the 1.2.1 tag to the master branch until after you had already pushed this.
That is very odd - the commit dates all show a normal sequence of committing. I can only imagine that perhaps we forgot to set the "deny merge" bit in the git repo config and I wasn't up2date when I pushed ? The git config looks OK right now, but I see you modified it last night, so did you fix something ?
I had to temporarily disable the "deny merge" to push the merge that fixed things, but turned it back on after fixing things and verified that the hooks were indeed working as expected. So I don't know how we got in that state; oh well. By the way, what's the best way to test libvirt-python.git against uninstalled libvirt.git? I'm trying to add bindings for my virConnectDomainQemuMonitorEvent patches, but as my system installed libvirt is not yet at 1.2.2, I'm not sure the python bindings are building my new code. Could we expand HACKING to give better instructions on how to set up a build pointing to uninstalled libvirt? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Feb 04, 2014 at 06:31:26AM -0700, Eric Blake wrote:
On 02/04/2014 03:21 AM, Daniel P. Berrange wrote:
On Mon, Feb 03, 2014 at 02:32:27PM -0700, Eric Blake wrote:
On 01/23/2014 07:25 AM, Daniel P. Berrange wrote:
Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper.
Reported-by: Robie Basak <robie.basak@canonical.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Hmm, you pushed this patch onto the release before 1.2.1; I had to push a merge commit to get the master branch to include both this and the release. I'm not sure why git let you push a non-fast-forward, unless DV forgot to push the 1.2.1 tag to the master branch until after you had already pushed this.
That is very odd - the commit dates all show a normal sequence of committing. I can only imagine that perhaps we forgot to set the "deny merge" bit in the git repo config and I wasn't up2date when I pushed ? The git config looks OK right now, but I see you modified it last night, so did you fix something ?
I had to temporarily disable the "deny merge" to push the merge that fixed things, but turned it back on after fixing things and verified that the hooks were indeed working as expected. So I don't know how we got in that state; oh well.
By the way, what's the best way to test libvirt-python.git against uninstalled libvirt.git? I'm trying to add bindings for my virConnectDomainQemuMonitorEvent patches, but as my system installed libvirt is not yet at 1.2.2, I'm not sure the python bindings are building my new code. Could we expand HACKING to give better instructions on how to set up a build pointing to uninstalled libvirt?
You can change the prefix of the upstream libvirt, install it and then run setup.py with PKG_CONFIG_PATH changed. Or you can apply my patch which was NACKed in this discussion: http://www.redhat.com/archives/libvir-list/2013-November/msg01127.html Martin
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Martin Kletzander -
Osier Yang