Re: [PATCH 1/2] python: treat flags as default argument with value 0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/22/2013 11:31 AM, Eric Blake wrote:
On 03/21/2013 09:24 PM, Guannan Ren wrote:

migrate(self, dconn, flags, dname, uri, bandwidth):
migrate2(self, dconn, dxml, flags, dname, uri, bandwidth):
migrateToURI(self, duri, flags, dname, bandwidth):
migrateToURI2(self, dconnuri, miguri, dxml, flags, dname, bandwidth):
So how are they filtered? ...

           If we add flags=0 to above four APIs, we have to move the
flags arguments
           to the last position in the arguments list because the rule
"default arguments
           have to come after positional arguments." Changing them will
break the binding
           APIs. so I didn't touch them.
Actually, we should probably use

migrate(self, dconn, flags=0, dname=None, uri=None, bandwidth=0)

with sane defaults for all arguments after the flags.  After all, the C
api states:

  * virDomainMigrate:
  * @domain: a domain object
  * @dconn: destination host (a connection object)
  * @flags: bitwise-OR of virDomainMigrateFlags
  * @dname: (optional) rename domain to this at destination
  * @uri: (optional) dest hostname/URI as seen from the source host
  * @bandwidth: (optional) specify migration bandwidth limit in Mbps

But I'm okay if you change the migrate* functions in a separate patch,
since it will be touching more than just flags.

       okay.



...As I see you write "flags=0" for all the automatically generated
APIs here? And is there any risk to have other APIs of which flags
doesn't default to 0? Except the ones you mentioned in commit log.


        Yes,  I am not sure if the 0 is appropriatefor every APIs.
        I need more advice here.
        According to my test, they can accept the 0 value all.
flags == 0 should be sane for all APIs that we add.  In fact, for many
APIs, flags == 0 is the only value that we actually support, when we
haven't yet used any flags.


       Okay, thank you guys for the review.
   I   Pushed.

       Guannan


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]