Patchwork [1/7] iohandlers: Mark current implementation as 'old'

login
register
mail settings
Submitter Amit Shah
Date Feb. 22, 2011, 10:18 a.m.
Message ID <689bf4c031ed209bb306dacdd575b0a9bdb89cc5.1298369272.git.amit.shah@redhat.com>
Download mbox | patch
Permalink /patch/83949/
State New
Headers show

Comments

Amit Shah - Feb. 22, 2011, 10:18 a.m.
Mark the current iohandler list as 'old'.  In the next commit we'll
introduce a new iohandler api that will replace the list name.

The 'old' list will eventually be completely replaced by the new
implementation.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 vl.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
Avi Kivity - Feb. 22, 2011, 11:09 a.m.
On 02/22/2011 12:18 PM, Amit Shah wrote:
> Mark the current iohandler list as 'old'.  In the next commit we'll
> introduce a new iohandler api that will replace the list name.
>
> The 'old' list will eventually be completely replaced by the new
> implementation.
>

A better way to do this is to implement the old API in terms of the new 
API.  This ensures you don't lose any functionality, and reduces the 
amount of low-level infrastructure.
Amit Shah - Feb. 22, 2011, 11:17 a.m.
On (Tue) 22 Feb 2011 [13:09:34], Avi Kivity wrote:
> On 02/22/2011 12:18 PM, Amit Shah wrote:
> >Mark the current iohandler list as 'old'.  In the next commit we'll
> >introduce a new iohandler api that will replace the list name.
> >
> >The 'old' list will eventually be completely replaced by the new
> >implementation.
> 
> A better way to do this is to implement the old API in terms of the
> new API.  This ensures you don't lose any functionality, and reduces
> the amount of low-level infrastructure.

With this new approach of switching to just one callback, that is more
work for little gain as we'll just deprecate the older API soon.  (The
previous patches used this approach, btw.)

Would you still prefer to have an old->new mapping?

		Amit
Avi Kivity - Feb. 22, 2011, 1:28 p.m.
On 02/22/2011 01:17 PM, Amit Shah wrote:
> On (Tue) 22 Feb 2011 [13:09:34], Avi Kivity wrote:
> >  On 02/22/2011 12:18 PM, Amit Shah wrote:
> >  >Mark the current iohandler list as 'old'.  In the next commit we'll
> >  >introduce a new iohandler api that will replace the list name.
> >  >
> >  >The 'old' list will eventually be completely replaced by the new
> >  >implementation.
> >
> >  A better way to do this is to implement the old API in terms of the
> >  new API.  This ensures you don't lose any functionality, and reduces
> >  the amount of low-level infrastructure.
>
> With this new approach of switching to just one callback, that is more
> work for little gain as we'll just deprecate the older API soon.  (The
> previous patches used this approach, btw.)

Does "deprecate" mean "completely remove"?

If so I agree.

> Would you still prefer to have an old->new mapping?
>

If you don't remove the old API, yes.
Amit Shah - Feb. 22, 2011, 2:37 p.m.
On (Tue) 22 Feb 2011 [15:28:59], Avi Kivity wrote:
> On 02/22/2011 01:17 PM, Amit Shah wrote:
> >On (Tue) 22 Feb 2011 [13:09:34], Avi Kivity wrote:
> >>  On 02/22/2011 12:18 PM, Amit Shah wrote:
> >>  >Mark the current iohandler list as 'old'.  In the next commit we'll
> >>  >introduce a new iohandler api that will replace the list name.
> >>  >
> >>  >The 'old' list will eventually be completely replaced by the new
> >>  >implementation.
> >>
> >>  A better way to do this is to implement the old API in terms of the
> >>  new API.  This ensures you don't lose any functionality, and reduces
> >>  the amount of low-level infrastructure.
> >
> >With this new approach of switching to just one callback, that is more
> >work for little gain as we'll just deprecate the older API soon.  (The
> >previous patches used this approach, btw.)
> 
> Does "deprecate" mean "completely remove"?
> 
> If so I agree.

Yes, completely removing the older API is the plan (as noted in
message 00).

		Amit
Avi Kivity - Feb. 22, 2011, 2:43 p.m.
On 02/22/2011 04:37 PM, Amit Shah wrote:
> On (Tue) 22 Feb 2011 [15:28:59], Avi Kivity wrote:
> >  On 02/22/2011 01:17 PM, Amit Shah wrote:
> >  >On (Tue) 22 Feb 2011 [13:09:34], Avi Kivity wrote:
> >  >>   On 02/22/2011 12:18 PM, Amit Shah wrote:
> >  >>   >Mark the current iohandler list as 'old'.  In the next commit we'll
> >  >>   >introduce a new iohandler api that will replace the list name.
> >  >>   >
> >  >>   >The 'old' list will eventually be completely replaced by the new
> >  >>   >implementation.
> >  >>
> >  >>   A better way to do this is to implement the old API in terms of the
> >  >>   new API.  This ensures you don't lose any functionality, and reduces
> >  >>   the amount of low-level infrastructure.
> >  >
> >  >With this new approach of switching to just one callback, that is more
> >  >work for little gain as we'll just deprecate the older API soon.  (The
> >  >previous patches used this approach, btw.)
> >
> >  Does "deprecate" mean "completely remove"?
> >
> >  If so I agree.
>
> Yes, completely removing the older API is the plan (as noted in
> message 00).

Great.  That's a much better deprecate than "add the new thing and let 
the old thing rot" (which I've been guilty of a few times).

Patch

diff --git a/vl.c b/vl.c
index b436952..e248ec4 100644
--- a/vl.c
+++ b/vl.c
@@ -1037,8 +1037,8 @@  typedef struct IOHandlerRecord {
     QLIST_ENTRY(IOHandlerRecord) next;
 } IOHandlerRecord;
 
-static QLIST_HEAD(, IOHandlerRecord) io_handlers =
-    QLIST_HEAD_INITIALIZER(io_handlers);
+static QLIST_HEAD(, IOHandlerRecord) io_handlers_old =
+    QLIST_HEAD_INITIALIZER(io_handlers_old);
 
 
 /* XXX: fd_read_poll should be suppressed, but an API change is
@@ -1052,19 +1052,19 @@  int qemu_set_fd_handler2(int fd,
     IOHandlerRecord *ioh;
 
     if (!fd_read && !fd_write) {
-        QLIST_FOREACH(ioh, &io_handlers, next) {
+        QLIST_FOREACH(ioh, &io_handlers_old, next) {
             if (ioh->fd == fd) {
                 ioh->deleted = 1;
                 break;
             }
         }
     } else {
-        QLIST_FOREACH(ioh, &io_handlers, next) {
+        QLIST_FOREACH(ioh, &io_handlers_old, next) {
             if (ioh->fd == fd)
                 goto found;
         }
         ioh = qemu_mallocz(sizeof(IOHandlerRecord));
-        QLIST_INSERT_HEAD(&io_handlers, ioh, next);
+        QLIST_INSERT_HEAD(&io_handlers_old, ioh, next);
     found:
         ioh->fd = fd;
         ioh->fd_read_poll = fd_read_poll;
@@ -1347,7 +1347,7 @@  void main_loop_wait(int nonblocking)
     FD_ZERO(&rfds);
     FD_ZERO(&wfds);
     FD_ZERO(&xfds);
-    QLIST_FOREACH(ioh, &io_handlers, next) {
+    QLIST_FOREACH(ioh, &io_handlers_old, next) {
         if (ioh->deleted)
             continue;
         if (ioh->fd_read &&
@@ -1375,7 +1375,7 @@  void main_loop_wait(int nonblocking)
     if (ret > 0) {
         IOHandlerRecord *pioh;
 
-        QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
+        QLIST_FOREACH_SAFE(ioh, &io_handlers_old, next, pioh) {
             if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
                 ioh->fd_read(ioh->opaque);
             }