Hello Graham,

I have now taken the time to review this, and in short, it is over almost 3600 
lines of changes, which would be a very big job to intergrate into the 
current code base, make work, document, and ensure the stability of the code, 
so as it stands, we cannot accept it for inclusion into Bacula.  Please see 
below for how to get it included ...

As I have mentioned before, it is very hard to see the utility of this feature 
since bscan should not be needed very often -- at most once a year.  It seems 
to me that if this feature is important, either:
1. You have set your Volume retention periods too short.
2. You have not backed up your catalog
3. You are attempting to use scanning as a means to populate more than one 
catalog.

The first two, in my opinion are operational problems.  The third item above 
*is* very important, but for some time now, Eric and I have been discussing 
an SQL independent way of moving information between Bacula catalogs. This 
can be very important for: Archiving, Controlling multiple Directors, 
switching from one SQL engine to another, disaster recovery, ...
We believe the solution to this problem is a more generic way of exporting and 
importing Bacula catalog information (in an ASCII format), which will be much 
faster an much cleaner than scanning Volumes.

Now, here are some of the disadvantages of the current submission (so you can 
understand where I am coming from):
1. It is a very large change
2. Much of the new code does not follow our coding conventions
3. There is little or no documentation
4. It adds a lot of code to the SD, and as I mentioned above, I don't see the 
big need for this feature.
5. You are asking us to do several months of work to look at "all the pointers 
relating to jcrs, dcrs, devs, and so on in the storage daemon are supposed to 
be allocated and locked and how they interact and so on. If somebody could 
look at this and finish it off it would be great."  This is definitely not 
trivial.
6. Any problem with this very complex code (I mostly mean bscan rather than 
what you added) could very easily cause the whole SD to crash.  bscan is a 
very complicated piece of code.  It is sort of like a Virtual Machine that 
has to re-run all the jobs and deduce what they were doing by what is stored 
on the Volume and re-submit all the database commands they probably did.

That said, we can see the utility of your approach, so here are the conditions 
under which we can accept it:

1. Keep the bulk of the code in bscan rather than add it to the SD.
2. Keep a minimal "control" interface in the SD, and when the SD wants to do a 
scan, it frees up a drive and calls bscan with all the information it needs. 
There may be additional functionality needed if bscan wants to use an 
Autochanger, but we can work that out.
3. Either the SD communicates with bscan via the network, or the SD passes the
Director connection on to bscan.
4. The SD may need to pass a good amount of information to bscan via a file or 
by a network connection.
5. The code would be resubmitted with absolute minimal changes to the SD.
6. The code should strictly follow the Bacula programming style guidelines.
7. We would like some initial documentation with the code -- probably a page.
8. A commitment to provide all the necessary LaTeX documentation before the 
code is released.
9. A commitment to fix bugs in the new code for some period of time (a year 
would be nice).

Now the above, particularly keeping all the code in bscan complicates things 
for a developer, but it will ensure stability of the SD by keeping all 
the "dangerous" code in another process.  There are a number of details that 
need to be worked out for how to communicate between the SD and bscan.

Please note that we will be imposing a feature freeze somewhere around 
mid-to-late Novemeber for version 3.2.0, which we hope will be released 
during the month of January 2010.


If you interest really is to move information from one catalog to another, 
then we suggest you talk to us about working on our catalog import/export 
project.

Please let me know what you think.

Best regards,

Kern

PS: I looked at the documentation I added to the Developer's manual about how 
to submit patches, and it is really not at all adequate, so over the next 
week, I will be improving it to provide a "line-by-line" type description on 
how to work with git to make changes and submit patches.  


On Wednesday 28 October 2009 10:08:30 Graham Keeling wrote:
> On Fri, Oct 09, 2009 at 09:39:55PM +0200, Kern Sibbald wrote:
> > On Wednesday 07 October 2009 11:02:23 Graham Keeling wrote:
> > > Item  n: Run bscan on a remote storage daemon from within bconsole.
> > >   Date:  07 October 2009
> > >   Origin: Graham Keeling <gra...@equiinet.com>
> > >   Status: Proposing
> > >
> > >   What:  The ability to be able to run bscan on a remote storage daemon
> > > from within bconsole in order to populate your catalog.
> > >
> > >   Why:   Currently, it seems you have to:
> > >          a) log in to a console on the remote machine
> > >          b) figure out where the storage daemon config file is
> > >          c) figure out the storage device from the config file
> > >          d) figure out the catalog IP address
> > >          e) figure out the catalog port
> > >          f) open the port on the catalog firewall
> > >          g) configure the catalog database to accept connections from
> > > the remote host
> > >          h) build a 'bscan' command from (b)-(e) above and run it
> > >          It would be much nicer to be able to type something like this
> > > into bconsole:
> > >          *bscan storage=<storage> device=<device> volume=<volume>
> > >          or something like:
> > >          *bscan storage=<storage> all
> > >          It seems to me that the scan could also do a better job than
> > > the external bscan program currently does. It would possibly be able to
> > > deduce some extra details, such as the catalog StorageId for the
> > > volumes.
> > >
> > >   Notes:
> >
> > I have added this to the projects file, but with the following
> > reservations:
> >
> >   Notes: (Kern). If you need to do a bscan, you have done something
> > wrong, so this functionality should not need to be integrated into the
> > the Storage daemon.  However, I am not opposed to someone implementing
> > this feature providing that all the code is in a shared object (or dll)
> > and does not add significantly to the size of the Storage daemon. In
> > addition, the code should be written in a way such that the same source
> > code is used in both the bscan program and the Storage daemon to avoid
> > adding a lot of new code that must be maintained by the project.
>
> I have a patch for bacula-3.0.3 that does most of the work for this - it
> does queries to the director from bscan if you are running from the storage
> daemon, and direct database queries if you are using the standalone bscan.
>
> However, I cannot understand how all the pointers relating to jcrs, dcrs,
> devs, and so on in the storage daemon are supposed to be allocated and
> locked and how they interact and so on. If somebody could look at this and
> finish it off it would be great. I guess the bit to look at is bscan_cmd()
> in
> src/stored/dircmd.c.
>
> Also, I know that git is supposed to be used now, but I don't know how
> patches such as this are supposed to be submitted via it.
> If things have to be done via git, it would be nice to be told exactly what
> git commands I need to run to do the equivalent of submitting a patch.



------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Bacula-devel mailing list
Bacula-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bacula-devel

Reply via email to